2021-09-11 09:24:13

by Guo Ren

[permalink] [raw]
Subject: [RFC PATCH V4 0/6] riscv: Add PBMT & DMA for D1 bringup

From: Guo Ren <[email protected]>

These patches are a continuation of "riscv: Add DMA_COHERENT support for
Allwinner D1". In this version, we rebase on Atish's Global dma pool
patchset, and it has been tested on qemu and our hardware platforms.
But we append "select DMA_DIRECT_REMAP" in RISCV_DMA_NONCOHERENT, so not
sure it would affect Atish's hardware platform.

We still use riscv_dma_cache_sync_set, not the alternative code. I agree
the alternative framework is better for performance in dma ops. Maybe
Atish's next version of the patch would use it.

The custom PBMT implementation is moved into errata and add
apply_errata_setup_vm() in setup_vm prologue. Hope it could be approved.

You can follow the D1 fedora wiki[1], try the latest kernel with the
patchset.
[1] https://fedoraproject.org/wiki/Architectures/RISC-V/Allwinner#Build_Linux_Kernel_for_D1

Previous versions:
V3: https://lore.kernel.org/linux-riscv/[email protected]/
V2: https://lore.kernel.org/linux-riscv/[email protected]/
V1: https://lore.kernel.org/linux-riscv/[email protected]/

Atish Patra (2):
RISC-V: Support a new config option for non-coherent DMA
RISC-V: Implement arch_sync_dma* functions

Guo Ren (3):
riscv: pgtable: Add custom protection_map init
riscv: errata: pgtable: Add custom Svpbmt supported for Allwinner D1
riscv: errata: Support T-HEAD custom dcache ops

Liu Shaohua (1):
riscv: soc: Add Allwinner SoC kconfig option

arch/riscv/Kconfig | 13 +++
arch/riscv/Kconfig.erratas | 11 +++
arch/riscv/Kconfig.socs | 15 ++++
arch/riscv/configs/defconfig | 1 +
arch/riscv/errata/Makefile | 1 +
arch/riscv/errata/alternative.c | 23 +++++
arch/riscv/errata/thead/Makefile | 1 +
arch/riscv/errata/thead/errata.c | 108 +++++++++++++++++++++++
arch/riscv/include/asm/alternative.h | 4 +
arch/riscv/include/asm/dma-noncoherent.h | 19 ++++
arch/riscv/include/asm/fixmap.h | 2 +-
arch/riscv/include/asm/pgtable-64.h | 8 +-
arch/riscv/include/asm/pgtable-bits.h | 46 +++++++++-
arch/riscv/include/asm/pgtable.h | 30 ++++---
arch/riscv/include/asm/vendorid_list.h | 1 +
arch/riscv/mm/Makefile | 1 +
arch/riscv/mm/dma-noncoherent.c | 66 ++++++++++++++
arch/riscv/mm/init.c | 28 ++++++
mm/mmap.c | 4 +
19 files changed, 366 insertions(+), 16 deletions(-)
create mode 100644 arch/riscv/errata/thead/Makefile
create mode 100644 arch/riscv/errata/thead/errata.c
create mode 100644 arch/riscv/include/asm/dma-noncoherent.h
create mode 100644 arch/riscv/mm/dma-noncoherent.c

--
2.25.1


2021-09-11 09:24:14

by Guo Ren

[permalink] [raw]
Subject: [RFC PATCH V4 2/6] riscv: errata: pgtable: Add custom Svpbmt supported for Allwinner D1

From: Guo Ren <[email protected]>

RISC-V Svpbmt is gradually maturing, the draft is:

Svpbmt PTE format:
| 63 | 62-61 | 60-8 | 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0
N MT RSW D A G U X W R V
^

Of the Reserved bits [63:54] in a leaf PTE, the high bit is already
allocated (as the N bit), so bits [62:61] are used as the MT (aka
MemType) field. This field specifies one of three memory types that
are close equivalents (or equivalent in effect) to the three main x86
and ARMv8 memory types - as shown in the following table.

RISC-V
Encoding &
MemType RISC-V Description
---------- ------------------------------------------------
00 - PMA Normal Cacheable, No change to implied PMA memory type
01 - NC Non-cacheable, idempotent, weakly-ordered Main Memory
10 - IO Non-cacheable, non-idempotent, strongly-ordered I/O memory
11 - Rsvd Reserved for future standard use

But T-HEAD C906 in Allwinner D1 has defined a custom Svpbmt:

T-HEAD C9xx PTE format:
| 63 | 62 | 61 | 60 | 59-8 | 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0
SO C B SH RSW D A G U X W R V
^ ^ ^ ^
BIT(63): SO - Strong Order
BIT(62): C - Cacheable
BIT(61): B - Bufferable
BIT(60): SH - Shareable

MT_MASK : [63 - 59]
MT_PMA : C + SH
MT_NC : (none)
MT_IO : SO

The patch not only implements the D1's PBMT extension but also
considers future scalability by errata framework.

We are trying to keep both below work together:
- "riscv spec acceptance policy" (Svpbmt extension in future)
- "Linux Keep real hardware work" (Allwinner D1's custom Svpbmt)

Signed-off-by: Guo Ren <[email protected]>
Signed-off-by: Liu Shaohua <[email protected]>
Signed-off-by: Wei Fu <[email protected]>
Cc: Palmer Dabbelt <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Anup Patel <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Drew Fustini <[email protected]>
Cc: Wei Fu <[email protected]>
Cc: Wei Wu <[email protected]>
Cc: Chen-Yu Tsai <[email protected]>
Cc: Maxime Ripard <[email protected]>
Cc: Daniel Lustig <[email protected]>
Cc: Greg Favor <[email protected]>
Cc: Andrea Mondelli <[email protected]>
Cc: Jonathan Behrens <[email protected]>
Cc: Xinhaoqu (Freddie) <[email protected]>
Cc: Bill Huffman <[email protected]>
Cc: Nick Kossifidis <[email protected]>
Cc: Allen Baum <[email protected]>
Cc: Josh Scheid <[email protected]>
Cc: Richard Trauben <[email protected]>
---
arch/riscv/Kconfig.erratas | 11 ++++++
arch/riscv/errata/Makefile | 1 +
arch/riscv/errata/alternative.c | 18 ++++++++++
arch/riscv/errata/thead/Makefile | 1 +
arch/riscv/errata/thead/errata.c | 47 ++++++++++++++++++++++++++
arch/riscv/include/asm/alternative.h | 2 ++
arch/riscv/include/asm/fixmap.h | 2 +-
arch/riscv/include/asm/pgtable-64.h | 8 +++--
arch/riscv/include/asm/pgtable-bits.h | 46 +++++++++++++++++++++++--
arch/riscv/include/asm/pgtable.h | 30 ++++++++++------
arch/riscv/include/asm/vendorid_list.h | 1 +
arch/riscv/mm/init.c | 6 ++++
12 files changed, 157 insertions(+), 16 deletions(-)
create mode 100644 arch/riscv/errata/thead/Makefile
create mode 100644 arch/riscv/errata/thead/errata.c

diff --git a/arch/riscv/Kconfig.erratas b/arch/riscv/Kconfig.erratas
index b44d6ecdb46e..e130fd040494 100644
--- a/arch/riscv/Kconfig.erratas
+++ b/arch/riscv/Kconfig.erratas
@@ -41,4 +41,15 @@ config ERRATA_SIFIVE_CIP_1200

If you don't know what to do here, say "Y".

+config ERRATA_THEAD
+ bool "T-HEAD errata"
+ depends on RISCV_ERRATA_ALTERNATIVE
+ default y
+ help
+ All T-HEAD errata Kconfig depend on this Kconfig. Disabling
+ this Kconfig will disable all T-HEAD errata. Please say "Y"
+ here if your platform uses T-HEAD CPU cores.
+
+ Otherwise, please say "N" here to avoid unnecessary overhead.
+
endmenu
diff --git a/arch/riscv/errata/Makefile b/arch/riscv/errata/Makefile
index b8f8740a3e44..f6db15055e73 100644
--- a/arch/riscv/errata/Makefile
+++ b/arch/riscv/errata/Makefile
@@ -1,2 +1,3 @@
obj-y += alternative.o
obj-$(CONFIG_ERRATA_SIFIVE) += sifive/
+obj-$(CONFIG_ERRATA_THEAD) += thead/
diff --git a/arch/riscv/errata/alternative.c b/arch/riscv/errata/alternative.c
index 3b15885db70b..b879aa546bc5 100644
--- a/arch/riscv/errata/alternative.c
+++ b/arch/riscv/errata/alternative.c
@@ -72,3 +72,21 @@ void __init apply_boot_alternatives(void)
cpu_mfr_info.arch_id, cpu_mfr_info.imp_id);
}

+/*
+ * This is called very early form setup_vm in the boot process.
+ */
+void __init apply_errata_setup_vm(void)
+{
+ riscv_fill_cpu_mfr_info();
+
+ switch (cpu_mfr_info.vendor_id) {
+#ifdef CONFIG_ERRATA_THEAD
+ case THEAD_VENDOR_ID:
+ thead_errata_setup_vm(cpu_mfr_info.arch_id,
+ cpu_mfr_info.imp_id);
+ break;
+#endif
+ default:
+ break;
+ }
+}
diff --git a/arch/riscv/errata/thead/Makefile b/arch/riscv/errata/thead/Makefile
new file mode 100644
index 000000000000..2d644e19caef
--- /dev/null
+++ b/arch/riscv/errata/thead/Makefile
@@ -0,0 +1 @@
+obj-y += errata.o
diff --git a/arch/riscv/errata/thead/errata.c b/arch/riscv/errata/thead/errata.c
new file mode 100644
index 000000000000..1f5c0f82bc23
--- /dev/null
+++ b/arch/riscv/errata/thead/errata.c
@@ -0,0 +1,47 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/kernel.h>
+#include <linux/string.h>
+#include <linux/bug.h>
+#include <asm/patch.h>
+#include <asm/alternative.h>
+#include <asm/vendorid_list.h>
+#include <asm/errata_list.h>
+#include <asm/pgtable-bits.h>
+
+#ifdef CONFIG_64BIT
+/*
+ * T-HEAD C9xx PTE format:
+ * | 63 | 62 | 61 | 60 | 59-8 | 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0
+ * SO C B SH RSW D A G U X W R V
+ * ^ ^ ^ ^ ^
+ * BIT(63): SO - Strong Order
+ * BIT(62): C - Cacheable
+ * BIT(61): B - Bufferable
+ * BIT(60): SH - Shareable
+ *
+ * MT_MASK : [63 - 59]
+ * MT_PMA : C + SH
+ * MT_NC : (none)
+ * MT_IO : SO
+ */
+#undef _PAGE_MT_MASK
+#undef _PAGE_MT_PMA
+#undef _PAGE_MT_NC
+#undef _PAGE_MT_IO
+
+#define _PAGE_MT_MASK 0xf800000000000000
+#define _PAGE_MT_PMA 0x5000000000000000
+#define _PAGE_MT_NC 0x0
+#define _PAGE_MT_IO 0x8000000000000000
+#endif
+
+void __init thead_errata_setup_vm(unsigned long archid, unsigned long impid)
+{
+#ifdef CONFIG_64BIT
+ __riscv_pbmt.mask = _PAGE_MT_MASK;
+ __riscv_pbmt.mt[MT_PMA] = _PAGE_MT_PMA;
+ __riscv_pbmt.mt[MT_NC] = _PAGE_MT_NC;
+ __riscv_pbmt.mt[MT_IO] = _PAGE_MT_IO;
+#endif
+}
diff --git a/arch/riscv/include/asm/alternative.h b/arch/riscv/include/asm/alternative.h
index e625d3cafbed..3605894081a8 100644
--- a/arch/riscv/include/asm/alternative.h
+++ b/arch/riscv/include/asm/alternative.h
@@ -18,6 +18,7 @@
#include <asm/hwcap.h>

void __init apply_boot_alternatives(void);
+void __init apply_errata_setup_vm(void);

struct alt_entry {
void *old_ptr; /* address of original instruciton or data */
@@ -35,5 +36,6 @@ struct errata_checkfunc_id {
void sifive_errata_patch_func(struct alt_entry *begin, struct alt_entry *end,
unsigned long archid, unsigned long impid);

+void thead_errata_setup_vm(unsigned long archid, unsigned long impid);
#endif
#endif
diff --git a/arch/riscv/include/asm/fixmap.h b/arch/riscv/include/asm/fixmap.h
index 54cbf07fb4e9..5acd99d08e74 100644
--- a/arch/riscv/include/asm/fixmap.h
+++ b/arch/riscv/include/asm/fixmap.h
@@ -43,7 +43,7 @@ enum fixed_addresses {
__end_of_fixed_addresses
};

-#define FIXMAP_PAGE_IO PAGE_KERNEL
+#define FIXMAP_PAGE_IO PAGE_IOREMAP

#define __early_set_fixmap __set_fixmap

diff --git a/arch/riscv/include/asm/pgtable-64.h b/arch/riscv/include/asm/pgtable-64.h
index 228261aa9628..0b53ea67e91a 100644
--- a/arch/riscv/include/asm/pgtable-64.h
+++ b/arch/riscv/include/asm/pgtable-64.h
@@ -61,12 +61,14 @@ static inline void pud_clear(pud_t *pudp)

static inline pmd_t *pud_pgtable(pud_t pud)
{
- return (pmd_t *)pfn_to_virt(pud_val(pud) >> _PAGE_PFN_SHIFT);
+ return (pmd_t *)pfn_to_virt((pud_val(pud) & _PAGE_CHG_MASK)
+ >> _PAGE_PFN_SHIFT);
}

static inline struct page *pud_page(pud_t pud)
{
- return pfn_to_page(pud_val(pud) >> _PAGE_PFN_SHIFT);
+ return pfn_to_page((pud_val(pud) & _PAGE_CHG_MASK)
+ >> _PAGE_PFN_SHIFT);
}

static inline pmd_t pfn_pmd(unsigned long pfn, pgprot_t prot)
@@ -76,7 +78,7 @@ static inline pmd_t pfn_pmd(unsigned long pfn, pgprot_t prot)

static inline unsigned long _pmd_pfn(pmd_t pmd)
{
- return pmd_val(pmd) >> _PAGE_PFN_SHIFT;
+ return (pmd_val(pmd) & _PAGE_CHG_MASK) >> _PAGE_PFN_SHIFT;
}

#define mk_pmd(page, prot) pfn_pmd(page_to_pfn(page), prot)
diff --git a/arch/riscv/include/asm/pgtable-bits.h b/arch/riscv/include/asm/pgtable-bits.h
index 2ee413912926..38a33e076714 100644
--- a/arch/riscv/include/asm/pgtable-bits.h
+++ b/arch/riscv/include/asm/pgtable-bits.h
@@ -7,7 +7,7 @@
#define _ASM_RISCV_PGTABLE_BITS_H

/*
- * PTE format:
+ * rv32 PTE format:
* | XLEN-1 10 | 9 8 | 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0
* PFN reserved for SW D A G U X W R V
*/
@@ -24,6 +24,47 @@
#define _PAGE_DIRTY (1 << 7) /* Set by hardware on any write */
#define _PAGE_SOFT (1 << 8) /* Reserved for software */

+#ifndef __ASSEMBLY__
+#ifdef CONFIG_64BIT
+/*
+ * rv64 PTE format:
+ * | 63 | 62 61 | 60 54 | 53 10 | 9 8 | 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0
+ * N MT RSV PFN reserved for SW D A G U X W R V
+ * [62:61] Memory Type definitions:
+ * 00 - PMA Normal Cacheable, No change to implied PMA memory type
+ * 01 - NC Non-cacheable, idempotent, weakly-ordered Main Memory
+ * 10 - IO Non-cacheable, non-idempotent, strongly-ordered I/O memory
+ * 11 - Rsvd Reserved for future standard use
+ */
+#define _PAGE_MT_MASK ((u64)0x3 << 61)
+#define _PAGE_MT_PMA ((u64)0x0 << 61)
+#define _PAGE_MT_NC ((u64)0x1 << 61)
+#define _PAGE_MT_IO ((u64)0x2 << 61)
+
+enum {
+ MT_PMA,
+ MT_NC,
+ MT_IO,
+ MT_MAX
+};
+
+extern struct __riscv_pbmt_struct {
+ unsigned long mask;
+ unsigned long mt[MT_MAX];
+} __riscv_pbmt;
+
+#define _PAGE_DMA_MASK __riscv_pbmt.mask
+#define _PAGE_DMA_PMA __riscv_pbmt.mt[MT_PMA]
+#define _PAGE_DMA_NC __riscv_pbmt.mt[MT_NC]
+#define _PAGE_DMA_IO __riscv_pbmt.mt[MT_IO]
+#else
+#define _PAGE_DMA_MASK 0
+#define _PAGE_DMA_PMA 0
+#define _PAGE_DMA_NC 0
+#define _PAGE_DMA_IO 0
+#endif /* CONFIG_64BIT */
+#endif /* __ASSEMBLY__ */
+
#define _PAGE_SPECIAL _PAGE_SOFT
#define _PAGE_TABLE _PAGE_PRESENT

@@ -38,7 +79,8 @@
/* Set of bits to preserve across pte_modify() */
#define _PAGE_CHG_MASK (~(unsigned long)(_PAGE_PRESENT | _PAGE_READ | \
_PAGE_WRITE | _PAGE_EXEC | \
- _PAGE_USER | _PAGE_GLOBAL))
+ _PAGE_USER | _PAGE_GLOBAL | \
+ _PAGE_DMA_MASK))
/*
* when all of R/W/X are zero, the PTE is a pointer to the next level
* of the page table; otherwise, it is a leaf PTE.
diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
index 39b550310ec6..dcde56f2046e 100644
--- a/arch/riscv/include/asm/pgtable.h
+++ b/arch/riscv/include/asm/pgtable.h
@@ -115,7 +115,7 @@
#define USER_PTRS_PER_PGD (TASK_SIZE / PGDIR_SIZE)

/* Page protection bits */
-#define _PAGE_BASE (_PAGE_PRESENT | _PAGE_ACCESSED | _PAGE_USER)
+#define _PAGE_BASE (_PAGE_PRESENT | _PAGE_ACCESSED | _PAGE_USER | _PAGE_DMA_PMA)

#define PAGE_NONE __pgprot(_PAGE_PROT_NONE)
#define PAGE_READ __pgprot(_PAGE_BASE | _PAGE_READ)
@@ -136,7 +136,8 @@
| _PAGE_PRESENT \
| _PAGE_ACCESSED \
| _PAGE_DIRTY \
- | _PAGE_GLOBAL)
+ | _PAGE_GLOBAL \
+ | _PAGE_DMA_PMA)

#define PAGE_KERNEL __pgprot(_PAGE_KERNEL)
#define PAGE_KERNEL_READ __pgprot(_PAGE_KERNEL & ~_PAGE_WRITE)
@@ -146,11 +147,9 @@

#define PAGE_TABLE __pgprot(_PAGE_TABLE)

-/*
- * The RISC-V ISA doesn't yet specify how to query or modify PMAs, so we can't
- * change the properties of memory regions.
- */
-#define _PAGE_IOREMAP _PAGE_KERNEL
+#define _PAGE_IOREMAP ((_PAGE_KERNEL & ~_PAGE_DMA_MASK) | _PAGE_DMA_IO)
+
+#define PAGE_IOREMAP __pgprot(_PAGE_IOREMAP)

extern pgd_t swapper_pg_dir[];

@@ -230,12 +229,12 @@ static inline unsigned long _pgd_pfn(pgd_t pgd)

static inline struct page *pmd_page(pmd_t pmd)
{
- return pfn_to_page(pmd_val(pmd) >> _PAGE_PFN_SHIFT);
+ return pfn_to_page((pmd_val(pmd) & _PAGE_CHG_MASK) >> _PAGE_PFN_SHIFT);
}

static inline unsigned long pmd_page_vaddr(pmd_t pmd)
{
- return (unsigned long)pfn_to_virt(pmd_val(pmd) >> _PAGE_PFN_SHIFT);
+ return (unsigned long)pfn_to_virt((pmd_val(pmd) & _PAGE_CHG_MASK) >> _PAGE_PFN_SHIFT);
}

static inline pte_t pmd_pte(pmd_t pmd)
@@ -251,7 +250,7 @@ static inline pte_t pud_pte(pud_t pud)
/* Yields the page frame number (PFN) of a page table entry */
static inline unsigned long pte_pfn(pte_t pte)
{
- return (pte_val(pte) >> _PAGE_PFN_SHIFT);
+ return ((pte_val(pte) & _PAGE_CHG_MASK) >> _PAGE_PFN_SHIFT);
}

#define pte_page(x) pfn_to_page(pte_pfn(x))
@@ -490,6 +489,17 @@ static inline int ptep_clear_flush_young(struct vm_area_struct *vma,
return ptep_test_and_clear_young(vma, address, ptep);
}

+#define pgprot_noncached pgprot_noncached
+static inline pgprot_t pgprot_noncached(pgprot_t _prot)
+{
+ unsigned long prot = pgprot_val(_prot);
+
+ prot &= ~_PAGE_DMA_MASK;
+ prot |= _PAGE_DMA_IO;
+
+ return __pgprot(prot);
+}
+
/*
* THP functions
*/
diff --git a/arch/riscv/include/asm/vendorid_list.h b/arch/riscv/include/asm/vendorid_list.h
index 9d934215b3c8..bdfce1a4b42b 100644
--- a/arch/riscv/include/asm/vendorid_list.h
+++ b/arch/riscv/include/asm/vendorid_list.h
@@ -6,5 +6,6 @@
#define ASM_VENDOR_LIST_H

#define SIFIVE_VENDOR_ID 0x489
+#define THEAD_VENDOR_ID 0x5b7

#endif
diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index 975c7322d6c4..ee6dba4e49b5 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -577,6 +577,7 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
#ifndef __PAGETABLE_PMD_FOLDED
pmd_t fix_bmap_spmd, fix_bmap_epmd;
#endif
+ apply_errata_setup_vm();

setup_protection_map();

@@ -927,3 +928,8 @@ int __meminit vmemmap_populate(unsigned long start, unsigned long end, int node,
return vmemmap_populate_basepages(start, end, node, NULL);
}
#endif
+
+#ifdef CONFIG_64BIT
+struct __riscv_pbmt_struct __riscv_pbmt __ro_after_init;
+EXPORT_SYMBOL(__riscv_pbmt);
+#endif
--
2.25.1

2021-09-11 09:24:48

by Guo Ren

[permalink] [raw]
Subject: [RFC PATCH V4 3/6] RISC-V: Support a new config option for non-coherent DMA

From: Atish Patra <[email protected]>

In future, there will be more RISC-V platforms with non-coherent DMA.
Instead of selecting all the required config options in every soc, create
a new config that selects all the required configs related non-coherent
DMA.

Signed-off-by: Atish Patra <[email protected]>

RISC-V: Support DMA_DIRECT_REMAP for non-coherent DMA

Signed-off-by: Guo Ren <[email protected]>
---
arch/riscv/Kconfig | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 3ab3730bee92..d18a59ea10e5 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -213,6 +213,15 @@ config PGTABLE_LEVELS
config LOCKDEP_SUPPORT
def_bool y

+config RISCV_DMA_NONCOHERENT
+ bool
+ select ARCH_HAS_DMA_PREP_COHERENT
+ select ARCH_HAS_SYNC_DMA_FOR_DEVICE
+ select ARCH_HAS_SYNC_DMA_FOR_CPU
+ select ARCH_HAS_SETUP_DMA_OPS
+ select DMA_GLOBAL_POOL
+ select DMA_DIRECT_REMAP
+
source "arch/riscv/Kconfig.socs"
source "arch/riscv/Kconfig.erratas"

--
2.25.1

2021-09-11 09:26:48

by Guo Ren

[permalink] [raw]
Subject: [RFC PATCH V4 6/6] riscv: soc: Add Allwinner SoC kconfig option

From: Liu Shaohua <[email protected]>

Add Allwinner kconfig option which selects SoC specific and common
drivers that is required for this SoC.

Allwinner D1 uses custom PTE attributes to solve non-coherency SOC
interconnect issues for dma synchronization, so we set the default
value when SOC_SUNXI selected.

Signed-off-by: Liu Shaohua <[email protected]>
Signed-off-by: Guo Ren <[email protected]>
Signed-off-by: Wei Fu <[email protected]>
Cc: Anup Patel <[email protected]>
Cc: Atish Patra <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Chen-Yu Tsai <[email protected]>
Cc: Drew Fustini <[email protected]>
Cc: Maxime Ripard <[email protected]>
Cc: Palmer Dabbelt <[email protected]>
Cc: Wei Wu <[email protected]>
---
arch/riscv/Kconfig.socs | 15 +++++++++++++++
arch/riscv/configs/defconfig | 1 +
2 files changed, 16 insertions(+)

diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
index 30676ebb16eb..8721c000ef23 100644
--- a/arch/riscv/Kconfig.socs
+++ b/arch/riscv/Kconfig.socs
@@ -70,4 +70,19 @@ config SOC_CANAAN_K210_DTB_SOURCE

endif

+config SOC_SUNXI
+ bool "Allwinner SoCs"
+ depends on MMU
+ select DWMAC_GENERIC
+ select ERRATA_THEAD
+ select RISCV_DMA_NONCOHERENT
+ select RISCV_ERRATA_ALTERNATIVE
+ select SERIAL_8250
+ select SERIAL_8250_CONSOLE
+ select SERIAL_8250_DW
+ select SIFIVE_PLIC
+ select STMMAC_ETH
+ help
+ This enables support for Allwinner SoC platforms like the D1.
+
endmenu
diff --git a/arch/riscv/configs/defconfig b/arch/riscv/configs/defconfig
index bc68231a8fb7..a50f250fbdd8 100644
--- a/arch/riscv/configs/defconfig
+++ b/arch/riscv/configs/defconfig
@@ -15,6 +15,7 @@ CONFIG_BLK_DEV_INITRD=y
CONFIG_EXPERT=y
CONFIG_BPF_SYSCALL=y
CONFIG_SOC_SIFIVE=y
+CONFIG_SOC_SUNXI=y
CONFIG_SOC_VIRT=y
CONFIG_SOC_MICROCHIP_POLARFIRE=y
CONFIG_SMP=y
--
2.25.1

2021-09-11 09:27:12

by Guo Ren

[permalink] [raw]
Subject: [RFC PATCH V4 4/6] RISC-V: Implement arch_sync_dma* functions

From: Atish Patra <[email protected]>

To facilitate streaming DMA APIs, this patch introduces a set of generic
cache operations related dma sync. Any platform can use the generic ops
to provide platform specific cache management operations. Once the
standard RISC-V CMO extension is available, it can be built on top of it.

Below Added by Guo Ren:
1. Fixup arch_sync_dma_for_cpu with "add DMA_TO_DEVICE force" by Guo Ren and
follow the tips by Christoph:
/*
* Cache operations depending on function and direction argument, inspired by
* https://lkml.org/lkml/2018/5/18/979
* "dma_sync_*_for_cpu and direction=TO_DEVICE (was Re: [PATCH 02/20]
* dma-mapping: provide a generic dma-noncoherent implementation)"
*
* | map == for_device | unmap == for_cpu
* |----------------------------------------------------------------
* TO_DEV | writeback writeback | none none
* FROM_DEV | invalidate invalidate | invalidate* invalidate*
* BIDIR | writeback+inv writeback+inv | invalidate invalidate
*
* [*] needed for CPU speculative prefetches
*
* NOTE: we don't check the validity of direction argument as it is done in
* upper layer functions (in include/linux/dma-mapping.h)
*/

2. Christoph:
As told a bunch of times before: doing indirect calls here is a
performance nightmare. Use something that actually does perform
horribly like alternatives. Or even delay implementing that until
^^^^^^^^^^^^ Agree, and TODO in Atish next path?
we need it and do a plain direct call for now.

Signed-off-by: Atish Patra <[email protected]>
Signed-off-by: Guo Ren <[email protected]>
Cc: Christoph Hellwig <[email protected]>
---
arch/riscv/include/asm/dma-noncoherent.h | 19 +++++++
arch/riscv/mm/Makefile | 1 +
arch/riscv/mm/dma-noncoherent.c | 66 ++++++++++++++++++++++++
3 files changed, 86 insertions(+)
create mode 100644 arch/riscv/include/asm/dma-noncoherent.h
create mode 100644 arch/riscv/mm/dma-noncoherent.c

diff --git a/arch/riscv/include/asm/dma-noncoherent.h b/arch/riscv/include/asm/dma-noncoherent.h
new file mode 100644
index 000000000000..5bdb03c9c427
--- /dev/null
+++ b/arch/riscv/include/asm/dma-noncoherent.h
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2021 Western Digital Corporation or its affiliates.
+ */
+
+#ifndef __ASM_RISCV_DMA_NON_COHERENT_H
+#define __ASM_RISCV_DMA_NON_COHERENT_H
+
+#ifdef CONFIG_RISCV_DMA_NONCOHERENT
+struct riscv_dma_cache_sync {
+ void (*cache_invalidate)(phys_addr_t paddr, size_t size);
+ void (*cache_clean)(phys_addr_t paddr, size_t size);
+ void (*cache_flush)(phys_addr_t paddr, size_t size);
+};
+
+void riscv_dma_cache_sync_set(struct riscv_dma_cache_sync *ops);
+#endif
+
+#endif
diff --git a/arch/riscv/mm/Makefile b/arch/riscv/mm/Makefile
index 7ebaef10ea1b..959bef49098b 100644
--- a/arch/riscv/mm/Makefile
+++ b/arch/riscv/mm/Makefile
@@ -27,3 +27,4 @@ KASAN_SANITIZE_init.o := n
endif

obj-$(CONFIG_DEBUG_VIRTUAL) += physaddr.o
+obj-$(CONFIG_RISCV_DMA_NONCOHERENT) += dma-noncoherent.o
diff --git a/arch/riscv/mm/dma-noncoherent.c b/arch/riscv/mm/dma-noncoherent.c
new file mode 100644
index 000000000000..63134d57016c
--- /dev/null
+++ b/arch/riscv/mm/dma-noncoherent.c
@@ -0,0 +1,66 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * RISC-V specific functions to support DMA for non-coherent devices
+ *
+ * Copyright (c) 2021 Western Digital Corporation or its affiliates.
+ */
+
+#include <linux/dma-direct.h>
+#include <linux/dma-map-ops.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/libfdt.h>
+#include <linux/mm.h>
+#include <linux/of.h>
+#include <linux/of_fdt.h>
+#include <asm/dma-noncoherent.h>
+
+static struct riscv_dma_cache_sync *dma_cache_sync;
+unsigned long riscv_dma_uc_offset;
+
+static void __dma_sync(phys_addr_t paddr, size_t size, enum dma_data_direction dir)
+{
+ if ((dir == DMA_FROM_DEVICE) && (dma_cache_sync->cache_invalidate))
+ dma_cache_sync->cache_invalidate(paddr, size);
+ else if ((dir == DMA_TO_DEVICE) && (dma_cache_sync->cache_clean))
+ dma_cache_sync->cache_clean(paddr, size);
+ else if ((dir == DMA_BIDIRECTIONAL) && dma_cache_sync->cache_flush)
+ dma_cache_sync->cache_flush(paddr, size);
+}
+
+void arch_sync_dma_for_device(phys_addr_t paddr, size_t size, enum dma_data_direction dir)
+{
+ if (!dma_cache_sync)
+ return;
+
+ __dma_sync(paddr, size, dir);
+}
+
+void arch_sync_dma_for_cpu(phys_addr_t paddr, size_t size, enum dma_data_direction dir)
+{
+ if (!dma_cache_sync || dir == DMA_TO_DEVICE)
+ return;
+
+ __dma_sync(paddr, size, DMA_FROM_DEVICE);
+}
+
+void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
+ const struct iommu_ops *iommu, bool coherent)
+{
+ /* If a specific device is dma-coherent, set it here */
+ dev->dma_coherent = coherent;
+}
+
+void arch_dma_prep_coherent(struct page *page, size_t size)
+{
+ void *flush_addr = page_address(page);
+
+ memset(flush_addr, 0, size);
+ if (dma_cache_sync && dma_cache_sync->cache_flush)
+ dma_cache_sync->cache_flush(__pa(flush_addr), size);
+}
+
+void riscv_dma_cache_sync_set(struct riscv_dma_cache_sync *ops)
+{
+ dma_cache_sync = ops;
+}
--
2.25.1

2021-09-11 09:27:45

by Guo Ren

[permalink] [raw]
Subject: [RFC PATCH V4 5/6] riscv: errata: Support T-HEAD custom dcache ops

From: Guo Ren <[email protected]>

Here are the DMA sync ops needed by Allwinner D1. RISC-V CMO
extension is still in progress, and D1 is using custom CMO
instructions:

dcache.ipa rs1 (invalidate)
| 31 - 25 | 24 - 20 | 19 - 15 | 14 - 12 | 11 - 7 | 6 - 0 |
0000001 01010 rs1 000 00000 0001011

dcache.cpa rs1 (clean)
| 31 - 25 | 24 - 20 | 19 - 15 | 14 - 12 | 11 - 7 | 6 - 0 |
0000001 01001 rs1 000 00000 0001011

dcache.cipa rs1 (clean then invalidate)
| 31 - 25 | 24 - 20 | 19 - 15 | 14 - 12 | 11 - 7 | 6 - 0 |
0000001 01011 rs1 000 00000 0001011

sync.s (completion barrier)
| 31 - 25 | 24 - 20 | 19 - 15 | 14 - 12 | 11 - 7 | 6 - 0 |
0000000 11001 00000 000 00000 0001011

TODO:
- Using alternative patch_text based on Atish's patch.

Signed-off-by: Guo Ren <[email protected]>
Signed-off-by: Liu Shaohua <[email protected]>
Signed-off-by: Wei Fu <[email protected]>
Cc: Atish Patra <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Palmer Dabbelt <[email protected]>
Cc: Anup Patel <[email protected]>
---
arch/riscv/errata/alternative.c | 5 +++
arch/riscv/errata/thead/errata.c | 61 ++++++++++++++++++++++++++++
arch/riscv/include/asm/alternative.h | 2 +
3 files changed, 68 insertions(+)

diff --git a/arch/riscv/errata/alternative.c b/arch/riscv/errata/alternative.c
index b879aa546bc5..396aab1b62c2 100644
--- a/arch/riscv/errata/alternative.c
+++ b/arch/riscv/errata/alternative.c
@@ -46,6 +46,11 @@ static void __init init_alternative(void)
case SIFIVE_VENDOR_ID:
vendor_patch_func = sifive_errata_patch_func;
break;
+#endif
+#ifdef CONFIG_ERRATA_THEAD
+ case THEAD_VENDOR_ID:
+ vendor_patch_func = thead_errata_patch_func;
+ break;
#endif
default:
vendor_patch_func = NULL;
diff --git a/arch/riscv/errata/thead/errata.c b/arch/riscv/errata/thead/errata.c
index 1f5c0f82bc23..9c0bf9b25be3 100644
--- a/arch/riscv/errata/thead/errata.c
+++ b/arch/riscv/errata/thead/errata.c
@@ -5,6 +5,7 @@
#include <linux/bug.h>
#include <asm/patch.h>
#include <asm/alternative.h>
+#include <asm/dma-noncoherent.h>
#include <asm/vendorid_list.h>
#include <asm/errata_list.h>
#include <asm/pgtable-bits.h>
@@ -45,3 +46,63 @@ void __init thead_errata_setup_vm(unsigned long archid, unsigned long impid)
__riscv_pbmt.mt[MT_IO] = _PAGE_MT_IO;
#endif
}
+
+#ifdef CONFIG_RISCV_DMA_NONCOHERENT
+/*
+ * dcache.ipa rs1 (invalidate)
+ * | 31 - 25 | 24 - 20 | 19 - 15 | 14 - 12 | 11 - 7 | 6 - 0 |
+ * 0000001 01010 rs1 000 00000 0001011
+ *
+ * dcache.cpa rs1 (clean)
+ * | 31 - 25 | 24 - 20 | 19 - 15 | 14 - 12 | 11 - 7 | 6 - 0 |
+ * 0000001 01001 rs1 000 00000 0001011
+ *
+ * dcache.cipa rs1 (clean then invalidate)
+ * | 31 - 25 | 24 - 20 | 19 - 15 | 14 - 12 | 11 - 7 | 6 - 0 |
+ * 0000001 01011 rs1 000 00000 0001011
+ *
+ * sync.s
+ * | 31 - 25 | 24 - 20 | 19 - 15 | 14 - 12 | 11 - 7 | 6 - 0 |
+ * 0000000 11001 00000 000 00000 0001011
+ */
+#define DCACHE_IPA_A0 ".long 0x02a5000b"
+#define DCACHE_CPA_A0 ".long 0x0295000b"
+#define DCACHE_CIPA_A0 ".long 0x02b5000b"
+
+#define SYNC_S ".long 0x0190000b"
+
+#define CACHE_OP_RANGE(OP, start, size) \
+ register unsigned long i asm("a0") = start & ~(L1_CACHE_BYTES - 1); \
+ for (; i < ALIGN(start + size, L1_CACHE_BYTES); i += L1_CACHE_BYTES) \
+ __asm__ __volatile__(OP); \
+ __asm__ __volatile__(SYNC_S);
+
+static void c900_cache_invalidate(phys_addr_t start, size_t size)
+{
+ CACHE_OP_RANGE(DCACHE_IPA_A0, start, size);
+}
+
+static void c900_cache_clean(phys_addr_t start, size_t size)
+{
+ CACHE_OP_RANGE(DCACHE_CPA_A0, start, size);
+}
+
+static void c900_cache_flush(phys_addr_t start, size_t size)
+{
+ CACHE_OP_RANGE(DCACHE_CIPA_A0, start, size);
+}
+
+static struct riscv_dma_cache_sync c900_dma_cache_sync = {
+ .cache_invalidate = c900_cache_invalidate,
+ .cache_clean = c900_cache_clean,
+ .cache_flush = c900_cache_flush,
+};
+#endif
+
+void __init thead_errata_patch_func(struct alt_entry *begin, struct alt_entry *end,
+ unsigned long archid, unsigned long impid)
+{
+#ifdef CONFIG_RISCV_DMA_NONCOHERENT
+ riscv_dma_cache_sync_set(&c900_dma_cache_sync);
+#endif
+}
diff --git a/arch/riscv/include/asm/alternative.h b/arch/riscv/include/asm/alternative.h
index 3605894081a8..a519671fa7d1 100644
--- a/arch/riscv/include/asm/alternative.h
+++ b/arch/riscv/include/asm/alternative.h
@@ -35,6 +35,8 @@ struct errata_checkfunc_id {

void sifive_errata_patch_func(struct alt_entry *begin, struct alt_entry *end,
unsigned long archid, unsigned long impid);
+void thead_errata_patch_func(struct alt_entry *begin, struct alt_entry *end,
+ unsigned long archid, unsigned long impid);

void thead_errata_setup_vm(unsigned long archid, unsigned long impid);
#endif
--
2.25.1

2021-09-13 08:49:12

by Maxime Ripard

[permalink] [raw]
Subject: Re: [RFC PATCH V4 6/6] riscv: soc: Add Allwinner SoC kconfig option

Hi,

On Sat, Sep 11, 2021 at 05:21:39PM +0800, [email protected] wrote:
> From: Liu Shaohua <[email protected]>
>
> Add Allwinner kconfig option which selects SoC specific and common
> drivers that is required for this SoC.
>
> Allwinner D1 uses custom PTE attributes to solve non-coherency SOC
> interconnect issues for dma synchronization, so we set the default
> value when SOC_SUNXI selected.
>
> Signed-off-by: Liu Shaohua <[email protected]>
> Signed-off-by: Guo Ren <[email protected]>
> Signed-off-by: Wei Fu <[email protected]>
> Cc: Anup Patel <[email protected]>
> Cc: Atish Patra <[email protected]>
> Cc: Christoph Hellwig <[email protected]>
> Cc: Chen-Yu Tsai <[email protected]>
> Cc: Drew Fustini <[email protected]>
> Cc: Maxime Ripard <[email protected]>
> Cc: Palmer Dabbelt <[email protected]>
> Cc: Wei Wu <[email protected]>
> ---
> arch/riscv/Kconfig.socs | 15 +++++++++++++++
> arch/riscv/configs/defconfig | 1 +
> 2 files changed, 16 insertions(+)
>
> diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
> index 30676ebb16eb..8721c000ef23 100644
> --- a/arch/riscv/Kconfig.socs
> +++ b/arch/riscv/Kconfig.socs
> @@ -70,4 +70,19 @@ config SOC_CANAAN_K210_DTB_SOURCE
>
> endif
>
> +config SOC_SUNXI
> + bool "Allwinner SoCs"
> + depends on MMU
> + select DWMAC_GENERIC
> + select ERRATA_THEAD
> + select RISCV_DMA_NONCOHERENT
> + select RISCV_ERRATA_ALTERNATIVE
> + select SERIAL_8250
> + select SERIAL_8250_CONSOLE
> + select SERIAL_8250_DW
> + select SIFIVE_PLIC
> + select STMMAC_ETH
> + help
> + This enables support for Allwinner SoC platforms like the D1.
> +

I'm not sure we should select the drivers there. We could very well
imagine a board without UART, or even more so without ethernet.

These options should be in the defconfig.

Maxime


Attachments:
(No filename) (1.84 kB)
signature.asc (235.00 B)
Download all attachments

2021-09-13 09:23:49

by Guo Ren

[permalink] [raw]
Subject: Re: [RFC PATCH V4 6/6] riscv: soc: Add Allwinner SoC kconfig option

On Mon, Sep 13, 2021 at 4:45 PM Maxime Ripard <[email protected]> wrote:
>
> Hi,
>
> On Sat, Sep 11, 2021 at 05:21:39PM +0800, [email protected] wrote:
> > From: Liu Shaohua <[email protected]>
> >
> > Add Allwinner kconfig option which selects SoC specific and common
> > drivers that is required for this SoC.
> >
> > Allwinner D1 uses custom PTE attributes to solve non-coherency SOC
> > interconnect issues for dma synchronization, so we set the default
> > value when SOC_SUNXI selected.
> >
> > Signed-off-by: Liu Shaohua <[email protected]>
> > Signed-off-by: Guo Ren <[email protected]>
> > Signed-off-by: Wei Fu <[email protected]>
> > Cc: Anup Patel <[email protected]>
> > Cc: Atish Patra <[email protected]>
> > Cc: Christoph Hellwig <[email protected]>
> > Cc: Chen-Yu Tsai <[email protected]>
> > Cc: Drew Fustini <[email protected]>
> > Cc: Maxime Ripard <[email protected]>
> > Cc: Palmer Dabbelt <[email protected]>
> > Cc: Wei Wu <[email protected]>
> > ---
> > arch/riscv/Kconfig.socs | 15 +++++++++++++++
> > arch/riscv/configs/defconfig | 1 +
> > 2 files changed, 16 insertions(+)
> >
> > diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
> > index 30676ebb16eb..8721c000ef23 100644
> > --- a/arch/riscv/Kconfig.socs
> > +++ b/arch/riscv/Kconfig.socs
> > @@ -70,4 +70,19 @@ config SOC_CANAAN_K210_DTB_SOURCE
> >
> > endif
> >
> > +config SOC_SUNXI
> > + bool "Allwinner SoCs"
> > + depends on MMU
> > + select DWMAC_GENERIC
> > + select ERRATA_THEAD
> > + select RISCV_DMA_NONCOHERENT
> > + select RISCV_ERRATA_ALTERNATIVE
> > + select SERIAL_8250
> > + select SERIAL_8250_CONSOLE
> > + select SERIAL_8250_DW
> > + select SIFIVE_PLIC
> > + select STMMAC_ETH
> > + help
> > + This enables support for Allwinner SoC platforms like the D1.
> > +
>
> I'm not sure we should select the drivers there. We could very well
> imagine a board without UART, or even more so without ethernet.
We just want people could bring D1 up easier, 8250 is the basic component.


>
> These options should be in the defconfig.
>
> Maxime



--
Best Regards
Guo Ren

ML: https://lore.kernel.org/linux-csky/

2021-09-14 00:54:26

by Randy Dunlap

[permalink] [raw]
Subject: Re: [RFC PATCH V4 6/6] riscv: soc: Add Allwinner SoC kconfig option

On 9/13/21 2:20 AM, Guo Ren wrote:
> On Mon, Sep 13, 2021 at 4:45 PM Maxime Ripard <[email protected]> wrote:
>>
>> Hi,
>>
>> On Sat, Sep 11, 2021 at 05:21:39PM +0800, [email protected] wrote:
>>> From: Liu Shaohua <[email protected]>
>>>
>>> Add Allwinner kconfig option which selects SoC specific and common
>>> drivers that is required for this SoC.
>>>
>>> Allwinner D1 uses custom PTE attributes to solve non-coherency SOC
>>> interconnect issues for dma synchronization, so we set the default
>>> value when SOC_SUNXI selected.
>>>
>>> Signed-off-by: Liu Shaohua <[email protected]>
>>> Signed-off-by: Guo Ren <[email protected]>
>>> Signed-off-by: Wei Fu <[email protected]>
>>> Cc: Anup Patel <[email protected]>
>>> Cc: Atish Patra <[email protected]>
>>> Cc: Christoph Hellwig <[email protected]>
>>> Cc: Chen-Yu Tsai <[email protected]>
>>> Cc: Drew Fustini <[email protected]>
>>> Cc: Maxime Ripard <[email protected]>
>>> Cc: Palmer Dabbelt <[email protected]>
>>> Cc: Wei Wu <[email protected]>
>>> ---
>>> arch/riscv/Kconfig.socs | 15 +++++++++++++++
>>> arch/riscv/configs/defconfig | 1 +
>>> 2 files changed, 16 insertions(+)
>>>
>>> diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
>>> index 30676ebb16eb..8721c000ef23 100644
>>> --- a/arch/riscv/Kconfig.socs
>>> +++ b/arch/riscv/Kconfig.socs
>>> @@ -70,4 +70,19 @@ config SOC_CANAAN_K210_DTB_SOURCE
>>>
>>> endif
>>>
>>> +config SOC_SUNXI
>>> + bool "Allwinner SoCs"
>>> + depends on MMU
>>> + select DWMAC_GENERIC
>>> + select ERRATA_THEAD
>>> + select RISCV_DMA_NONCOHERENT
>>> + select RISCV_ERRATA_ALTERNATIVE
>>> + select SERIAL_8250
>>> + select SERIAL_8250_CONSOLE
>>> + select SERIAL_8250_DW
>>> + select SIFIVE_PLIC
>>> + select STMMAC_ETH
>>> + help
>>> + This enables support for Allwinner SoC platforms like the D1.
>>> +
>>
>> I'm not sure we should select the drivers there. We could very well
>> imagine a board without UART, or even more so without ethernet.
> We just want people could bring D1 up easier, 8250 is the basic component.
>
>
>>
>> These options should be in the defconfig.

Agreed, using a defconfig is the right way to do this.

--
~Randy

2021-09-14 02:37:08

by Guo Ren

[permalink] [raw]
Subject: Re: [RFC PATCH V4 6/6] riscv: soc: Add Allwinner SoC kconfig option

On Tue, Sep 14, 2021 at 2:49 AM Randy Dunlap <[email protected]> wrote:
>
> On 9/13/21 2:20 AM, Guo Ren wrote:
> > On Mon, Sep 13, 2021 at 4:45 PM Maxime Ripard <[email protected]> wrote:
> >>
> >> Hi,
> >>
> >> On Sat, Sep 11, 2021 at 05:21:39PM +0800, [email protected] wrote:
> >>> From: Liu Shaohua <[email protected]>
> >>>
> >>> Add Allwinner kconfig option which selects SoC specific and common
> >>> drivers that is required for this SoC.
> >>>
> >>> Allwinner D1 uses custom PTE attributes to solve non-coherency SOC
> >>> interconnect issues for dma synchronization, so we set the default
> >>> value when SOC_SUNXI selected.
> >>>
> >>> Signed-off-by: Liu Shaohua <[email protected]>
> >>> Signed-off-by: Guo Ren <[email protected]>
> >>> Signed-off-by: Wei Fu <[email protected]>
> >>> Cc: Anup Patel <[email protected]>
> >>> Cc: Atish Patra <[email protected]>
> >>> Cc: Christoph Hellwig <[email protected]>
> >>> Cc: Chen-Yu Tsai <[email protected]>
> >>> Cc: Drew Fustini <[email protected]>
> >>> Cc: Maxime Ripard <[email protected]>
> >>> Cc: Palmer Dabbelt <[email protected]>
> >>> Cc: Wei Wu <[email protected]>
> >>> ---
> >>> arch/riscv/Kconfig.socs | 15 +++++++++++++++
> >>> arch/riscv/configs/defconfig | 1 +
> >>> 2 files changed, 16 insertions(+)
> >>>
> >>> diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
> >>> index 30676ebb16eb..8721c000ef23 100644
> >>> --- a/arch/riscv/Kconfig.socs
> >>> +++ b/arch/riscv/Kconfig.socs
> >>> @@ -70,4 +70,19 @@ config SOC_CANAAN_K210_DTB_SOURCE
> >>>
> >>> endif
> >>>
> >>> +config SOC_SUNXI
> >>> + bool "Allwinner SoCs"
> >>> + depends on MMU
> >>> + select DWMAC_GENERIC
> >>> + select ERRATA_THEAD
> >>> + select RISCV_DMA_NONCOHERENT
> >>> + select RISCV_ERRATA_ALTERNATIVE
> >>> + select SERIAL_8250
> >>> + select SERIAL_8250_CONSOLE
> >>> + select SERIAL_8250_DW
> >>> + select SIFIVE_PLIC
> >>> + select STMMAC_ETH
> >>> + help
> >>> + This enables support for Allwinner SoC platforms like the D1.
> >>> +
> >>
> >> I'm not sure we should select the drivers there. We could very well
> >> imagine a board without UART, or even more so without ethernet.
> > We just want people could bring D1 up easier, 8250 is the basic component.
> >
> >
> >>
> >> These options should be in the defconfig.
>
> Agreed, using a defconfig is the right way to do this.
Put 8250 related configs into arch/riscv/configs/defconfig?

@Palmer Dabbelt @Arnd Bergmann, How do you think about that?
(defconfig or Kconfig.soc)
My purpose is when people make the Image from riscv/defconfig, then
the Image could run on all platforms include D1.

>
> --
> ~Randy
>


--
Best Regards
Guo Ren

ML: https://lore.kernel.org/linux-csky/

2021-09-14 03:08:11

by Randy Dunlap

[permalink] [raw]
Subject: Re: [RFC PATCH V4 6/6] riscv: soc: Add Allwinner SoC kconfig option

On 9/13/21 7:34 PM, Guo Ren wrote:
> On Tue, Sep 14, 2021 at 2:49 AM Randy Dunlap <[email protected]> wrote:
>>
>> On 9/13/21 2:20 AM, Guo Ren wrote:
>>> On Mon, Sep 13, 2021 at 4:45 PM Maxime Ripard <[email protected]> wrote:
>>>>
>>>> Hi,
>>>>
>>>> On Sat, Sep 11, 2021 at 05:21:39PM +0800, [email protected] wrote:
>>>>> From: Liu Shaohua <[email protected]>
>>>>>
>>>>> Add Allwinner kconfig option which selects SoC specific and common
>>>>> drivers that is required for this SoC.
>>>>>
>>>>> Allwinner D1 uses custom PTE attributes to solve non-coherency SOC
>>>>> interconnect issues for dma synchronization, so we set the default
>>>>> value when SOC_SUNXI selected.
>>>>>
>>>>> Signed-off-by: Liu Shaohua <[email protected]>
>>>>> Signed-off-by: Guo Ren <[email protected]>
>>>>> Signed-off-by: Wei Fu <[email protected]>
>>>>> Cc: Anup Patel <[email protected]>
>>>>> Cc: Atish Patra <[email protected]>
>>>>> Cc: Christoph Hellwig <[email protected]>
>>>>> Cc: Chen-Yu Tsai <[email protected]>
>>>>> Cc: Drew Fustini <[email protected]>
>>>>> Cc: Maxime Ripard <[email protected]>
>>>>> Cc: Palmer Dabbelt <[email protected]>
>>>>> Cc: Wei Wu <[email protected]>
>>>>> ---
>>>>> arch/riscv/Kconfig.socs | 15 +++++++++++++++
>>>>> arch/riscv/configs/defconfig | 1 +
>>>>> 2 files changed, 16 insertions(+)
>>>>>
>>>>> diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
>>>>> index 30676ebb16eb..8721c000ef23 100644
>>>>> --- a/arch/riscv/Kconfig.socs
>>>>> +++ b/arch/riscv/Kconfig.socs
>>>>> @@ -70,4 +70,19 @@ config SOC_CANAAN_K210_DTB_SOURCE
>>>>>
>>>>> endif
>>>>>
>>>>> +config SOC_SUNXI
>>>>> + bool "Allwinner SoCs"
>>>>> + depends on MMU
>>>>> + select DWMAC_GENERIC
>>>>> + select ERRATA_THEAD
>>>>> + select RISCV_DMA_NONCOHERENT
>>>>> + select RISCV_ERRATA_ALTERNATIVE
>>>>> + select SERIAL_8250
>>>>> + select SERIAL_8250_CONSOLE
>>>>> + select SERIAL_8250_DW
>>>>> + select SIFIVE_PLIC
>>>>> + select STMMAC_ETH
>>>>> + help
>>>>> + This enables support for Allwinner SoC platforms like the D1.
>>>>> +
>>>>
>>>> I'm not sure we should select the drivers there. We could very well
>>>> imagine a board without UART, or even more so without ethernet.
>>> We just want people could bring D1 up easier, 8250 is the basic component.
>>>
>>>
>>>>
>>>> These options should be in the defconfig.
>>
>> Agreed, using a defconfig is the right way to do this.
> Put 8250 related configs into arch/riscv/configs/defconfig?
>
> @Palmer Dabbelt @Arnd Bergmann, How do you think about that?
> (defconfig or Kconfig.soc)
> My purpose is when people make the Image from riscv/defconfig, then
> the Image could run on all platforms include D1.

Hi,

I certainly did not understand your purpose with the patch being
able to build a kernel that would run on multiple platforms.
Still, I would not expect to see one platform cause unnecessary
drivers to be built for platforms that don't need them.

Kconfig.socs in arch/riscv/ is a bit of an unusual Kconfig file
IMO -- I suppose what you want to do fits into its style.

AFAIK the suggestion to use a defconfig (at least my suggestion)
was expecting to have a defconfig for each platform, but that
would not give you a boot image that could run on all platforms.

Anyway, it's Palmer's choice.

thanks.
--
~Randy

2021-09-14 03:50:52

by Heinrich Schuchardt

[permalink] [raw]
Subject: Re: [RFC PATCH V4 6/6] riscv: soc: Add Allwinner SoC kconfig option



On 9/13/21 10:45 AM, Maxime Ripard wrote:
> Hi,
>
> On Sat, Sep 11, 2021 at 05:21:39PM +0800, [email protected] wrote:
>> From: Liu Shaohua <[email protected]>
>>
>> Add Allwinner kconfig option which selects SoC specific and common
>> drivers that is required for this SoC.
>>
>> Allwinner D1 uses custom PTE attributes to solve non-coherency SOC
>> interconnect issues for dma synchronization, so we set the default
>> value when SOC_SUNXI selected.
>>
>> Signed-off-by: Liu Shaohua <[email protected]>
>> Signed-off-by: Guo Ren <[email protected]>
>> Signed-off-by: Wei Fu <[email protected]>
>> Cc: Anup Patel <[email protected]>
>> Cc: Atish Patra <[email protected]>
>> Cc: Christoph Hellwig <[email protected]>
>> Cc: Chen-Yu Tsai <[email protected]>
>> Cc: Drew Fustini <[email protected]>
>> Cc: Maxime Ripard <[email protected]>
>> Cc: Palmer Dabbelt <[email protected]>
>> Cc: Wei Wu <[email protected]>
>> ---
>> arch/riscv/Kconfig.socs | 15 +++++++++++++++
>> arch/riscv/configs/defconfig | 1 +
>> 2 files changed, 16 insertions(+)
>>
>> diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
>> index 30676ebb16eb..8721c000ef23 100644
>> --- a/arch/riscv/Kconfig.socs
>> +++ b/arch/riscv/Kconfig.socs
>> @@ -70,4 +70,19 @@ config SOC_CANAAN_K210_DTB_SOURCE
>>
>> endif
>>
>> +config SOC_SUNXI
>> + bool "Allwinner SoCs"
>> + depends on MMU
>> + select DWMAC_GENERIC
>> + select ERRATA_THEAD
>> + select RISCV_DMA_NONCOHERENT
>> + select RISCV_ERRATA_ALTERNATIVE
>> + select SERIAL_8250
>> + select SERIAL_8250_CONSOLE
>> + select SERIAL_8250_DW
>> + select SIFIVE_PLIC
>> + select STMMAC_ETH
>> + help
>> + This enables support for Allwinner SoC platforms like the D1.
>> +
>
> I'm not sure we should select the drivers there. We could very well
> imagine a board without UART, or even more so without ethernet.

The draft of the RISC-V platform specification is available here:
https://github.com/riscv/riscv-platform-specs/blob/main/riscv-platform-spec.adoc#uartserial-console

The specification requires in section "2.1.5.1. UART/Serial Console"
that on platforms with a rich operating system (e.g. Linux) you have a
serial console. Hence requiring 8250 support for the D1 CPU is justified.

In the riscv defconfig as of v5.14 we have:

CONFIG_SERIAL_8250=y
CONFIG_SERIAL_8250_CONSOLE=y
# CONFIG_SERIAL_8250_DW is not set
(Support for Synopsys DesignWare 8250 quirks)

CONFIG_SERIAL_8250_DW should be enabled (=y) in the defconfig.

As the specification requires a 16550 UART and marks 8250 as deprecated
I expect that future Allwinner SoCs will move to 16550. Calling a
Kconfig menu item "Allwinner SoCs" which includes all future Allwinner
SoCs irritates me. How about CONFIG_SOC_SUNXI_D1 instead?

Why does the patch use 'depends on MMU' and does not 'select MMU'?

Best regards

Heinrich

>
> These options should be in the defconfig.
>
> Maxime
>

2021-09-14 05:18:24

by Samuel Holland

[permalink] [raw]
Subject: Re: [RFC PATCH V4 6/6] riscv: soc: Add Allwinner SoC kconfig option

On 9/13/21 10:49 PM, Heinrich Schuchardt wrote:
> Calling a Kconfig menu item "Allwinner SoCs" which includes all
> future Allwinner SoCs irritates me. How about CONFIG_SOC_SUNXI_D1
> instead?

Would you want to have a separate option for each new SoC? That seems
like the only way to split things up, if you want to be more specific
than than "sunxi" (or equivalently "sun20i", which is the codename for
the RISC-V series).

Except at the very beginning (sun4i-sun7i), there have not been clear
generational boundaries between the various sunxi SoCs, so most of the
32-bit ones already get lumped into a single symbol (MACH_SUN8I). And
there is a single Kconfig symbol, ARCH_SUNXI, for all 64-bit Allwinner SoCs.

There is enough overlap in peripherals that you need a common symbol for
the peripheral drivers anyway. How about... ARCH_SUNXI? There are 90+
uses of this symbol throughout drivers/ and sound/, and I have found
that more than half of them apply to the D1 (see e.g. this commit[1] and
some of its ancestors).

RISC-V has so far adopted a CONFIG_SOC_xxx naming scheme, which is
different from arm/arm64's CONFIG_ARCH_xxx pattern. But now we have a
case where a SoC family is split between the two architectures. I'm all
for consistency with the names of other RISC-V platform symbols, but it
seems that reusing the existing ARCH_SUNXI symbol would be better than
cluttering up the driver Kconfig files with a duplicate.

Regards,
Samuel

[1]: https://github.com/smaeul/linux/commit/7841e5c32366

2021-09-14 05:21:04

by Anup Patel

[permalink] [raw]
Subject: Re: [RFC PATCH V4 6/6] riscv: soc: Add Allwinner SoC kconfig option

On Tue, Sep 14, 2021 at 8:36 AM Randy Dunlap <[email protected]> wrote:
>
> On 9/13/21 7:34 PM, Guo Ren wrote:
> > On Tue, Sep 14, 2021 at 2:49 AM Randy Dunlap <[email protected]> wrote:
> >>
> >> On 9/13/21 2:20 AM, Guo Ren wrote:
> >>> On Mon, Sep 13, 2021 at 4:45 PM Maxime Ripard <[email protected]> wrote:
> >>>>
> >>>> Hi,
> >>>>
> >>>> On Sat, Sep 11, 2021 at 05:21:39PM +0800, [email protected] wrote:
> >>>>> From: Liu Shaohua <[email protected]>
> >>>>>
> >>>>> Add Allwinner kconfig option which selects SoC specific and common
> >>>>> drivers that is required for this SoC.
> >>>>>
> >>>>> Allwinner D1 uses custom PTE attributes to solve non-coherency SOC
> >>>>> interconnect issues for dma synchronization, so we set the default
> >>>>> value when SOC_SUNXI selected.
> >>>>>
> >>>>> Signed-off-by: Liu Shaohua <[email protected]>
> >>>>> Signed-off-by: Guo Ren <[email protected]>
> >>>>> Signed-off-by: Wei Fu <[email protected]>
> >>>>> Cc: Anup Patel <[email protected]>
> >>>>> Cc: Atish Patra <[email protected]>
> >>>>> Cc: Christoph Hellwig <[email protected]>
> >>>>> Cc: Chen-Yu Tsai <[email protected]>
> >>>>> Cc: Drew Fustini <[email protected]>
> >>>>> Cc: Maxime Ripard <[email protected]>
> >>>>> Cc: Palmer Dabbelt <[email protected]>
> >>>>> Cc: Wei Wu <[email protected]>
> >>>>> ---
> >>>>> arch/riscv/Kconfig.socs | 15 +++++++++++++++
> >>>>> arch/riscv/configs/defconfig | 1 +
> >>>>> 2 files changed, 16 insertions(+)
> >>>>>
> >>>>> diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
> >>>>> index 30676ebb16eb..8721c000ef23 100644
> >>>>> --- a/arch/riscv/Kconfig.socs
> >>>>> +++ b/arch/riscv/Kconfig.socs
> >>>>> @@ -70,4 +70,19 @@ config SOC_CANAAN_K210_DTB_SOURCE
> >>>>>
> >>>>> endif
> >>>>>
> >>>>> +config SOC_SUNXI
> >>>>> + bool "Allwinner SoCs"
> >>>>> + depends on MMU
> >>>>> + select DWMAC_GENERIC
> >>>>> + select ERRATA_THEAD
> >>>>> + select RISCV_DMA_NONCOHERENT
> >>>>> + select RISCV_ERRATA_ALTERNATIVE
> >>>>> + select SERIAL_8250
> >>>>> + select SERIAL_8250_CONSOLE
> >>>>> + select SERIAL_8250_DW
> >>>>> + select SIFIVE_PLIC
> >>>>> + select STMMAC_ETH
> >>>>> + help
> >>>>> + This enables support for Allwinner SoC platforms like the D1.
> >>>>> +
> >>>>
> >>>> I'm not sure we should select the drivers there. We could very well
> >>>> imagine a board without UART, or even more so without ethernet.
> >>> We just want people could bring D1 up easier, 8250 is the basic component.
> >>>
> >>>
> >>>>
> >>>> These options should be in the defconfig.
> >>
> >> Agreed, using a defconfig is the right way to do this.
> > Put 8250 related configs into arch/riscv/configs/defconfig?
> >
> > @Palmer Dabbelt @Arnd Bergmann, How do you think about that?
> > (defconfig or Kconfig.soc)
> > My purpose is when people make the Image from riscv/defconfig, then
> > the Image could run on all platforms include D1.
>
> Hi,
>
> I certainly did not understand your purpose with the patch being
> able to build a kernel that would run on multiple platforms.
> Still, I would not expect to see one platform cause unnecessary
> drivers to be built for platforms that don't need them.
>
> Kconfig.socs in arch/riscv/ is a bit of an unusual Kconfig file
> IMO -- I suppose what you want to do fits into its style.
>
> AFAIK the suggestion to use a defconfig (at least my suggestion)
> was expecting to have a defconfig for each platform, but that
> would not give you a boot image that could run on all platforms.

AFAIK, having a separate defconfig for each platform is not going
to fly with distros (AFAIK). We can't expect dirstros to release
separate RISC-V kernel image for each platform. In fact, ARM64
kernel has just one defconfig whereas ARM32 kernel has
consolidated and minimized number of defconfigs.

The long term goal for Linux RISC-V is to support single kernel
image booting on multiple-platforms. Of course, users can always
strip down the kernel using their custom defconfigs.

Regards,
Anup

>
> Anyway, it's Palmer's choice.
>
> thanks.
> --
> ~Randy
>

2021-09-14 05:25:02

by Randy Dunlap

[permalink] [raw]
Subject: Re: [RFC PATCH V4 6/6] riscv: soc: Add Allwinner SoC kconfig option

On 9/13/21 10:16 PM, Anup Patel wrote:
> On Tue, Sep 14, 2021 at 8:36 AM Randy Dunlap <[email protected]> wrote:
>>
>> On 9/13/21 7:34 PM, Guo Ren wrote:
>>> On Tue, Sep 14, 2021 at 2:49 AM Randy Dunlap <[email protected]> wrote:
>>>>
>>>> On 9/13/21 2:20 AM, Guo Ren wrote:
>>>>> On Mon, Sep 13, 2021 at 4:45 PM Maxime Ripard <[email protected]> wrote:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> On Sat, Sep 11, 2021 at 05:21:39PM +0800, [email protected] wrote:
>>>>>>> From: Liu Shaohua <[email protected]>
>>>>>>>
>>>>>>> Add Allwinner kconfig option which selects SoC specific and common
>>>>>>> drivers that is required for this SoC.
>>>>>>>
>>>>>>> Allwinner D1 uses custom PTE attributes to solve non-coherency SOC
>>>>>>> interconnect issues for dma synchronization, so we set the default
>>>>>>> value when SOC_SUNXI selected.
>>>>>>>
>>>>>>> Signed-off-by: Liu Shaohua <[email protected]>
>>>>>>> Signed-off-by: Guo Ren <[email protected]>
>>>>>>> Signed-off-by: Wei Fu <[email protected]>
>>>>>>> Cc: Anup Patel <[email protected]>
>>>>>>> Cc: Atish Patra <[email protected]>
>>>>>>> Cc: Christoph Hellwig <[email protected]>
>>>>>>> Cc: Chen-Yu Tsai <[email protected]>
>>>>>>> Cc: Drew Fustini <[email protected]>
>>>>>>> Cc: Maxime Ripard <[email protected]>
>>>>>>> Cc: Palmer Dabbelt <[email protected]>
>>>>>>> Cc: Wei Wu <[email protected]>
>>>>>>> ---
>>>>>>> arch/riscv/Kconfig.socs | 15 +++++++++++++++
>>>>>>> arch/riscv/configs/defconfig | 1 +
>>>>>>> 2 files changed, 16 insertions(+)
>>>>>>>
>>>>>>> diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
>>>>>>> index 30676ebb16eb..8721c000ef23 100644
>>>>>>> --- a/arch/riscv/Kconfig.socs
>>>>>>> +++ b/arch/riscv/Kconfig.socs
>>>>>>> @@ -70,4 +70,19 @@ config SOC_CANAAN_K210_DTB_SOURCE
>>>>>>>
>>>>>>> endif
>>>>>>>
>>>>>>> +config SOC_SUNXI
>>>>>>> + bool "Allwinner SoCs"
>>>>>>> + depends on MMU
>>>>>>> + select DWMAC_GENERIC
>>>>>>> + select ERRATA_THEAD
>>>>>>> + select RISCV_DMA_NONCOHERENT
>>>>>>> + select RISCV_ERRATA_ALTERNATIVE
>>>>>>> + select SERIAL_8250
>>>>>>> + select SERIAL_8250_CONSOLE
>>>>>>> + select SERIAL_8250_DW
>>>>>>> + select SIFIVE_PLIC
>>>>>>> + select STMMAC_ETH
>>>>>>> + help
>>>>>>> + This enables support for Allwinner SoC platforms like the D1.
>>>>>>> +
>>>>>>
>>>>>> I'm not sure we should select the drivers there. We could very well
>>>>>> imagine a board without UART, or even more so without ethernet.
>>>>> We just want people could bring D1 up easier, 8250 is the basic component.
>>>>>
>>>>>
>>>>>>
>>>>>> These options should be in the defconfig.
>>>>
>>>> Agreed, using a defconfig is the right way to do this.
>>> Put 8250 related configs into arch/riscv/configs/defconfig?
>>>
>>> @Palmer Dabbelt @Arnd Bergmann, How do you think about that?
>>> (defconfig or Kconfig.soc)
>>> My purpose is when people make the Image from riscv/defconfig, then
>>> the Image could run on all platforms include D1.
>>
>> Hi,
>>
>> I certainly did not understand your purpose with the patch being
>> able to build a kernel that would run on multiple platforms.
>> Still, I would not expect to see one platform cause unnecessary
>> drivers to be built for platforms that don't need them.
>>
>> Kconfig.socs in arch/riscv/ is a bit of an unusual Kconfig file
>> IMO -- I suppose what you want to do fits into its style.
>>
>> AFAIK the suggestion to use a defconfig (at least my suggestion)
>> was expecting to have a defconfig for each platform, but that
>> would not give you a boot image that could run on all platforms.
>
> AFAIK, having a separate defconfig for each platform is not going
> to fly with distros (AFAIK). We can't expect dirstros to release
> separate RISC-V kernel image for each platform. In fact, ARM64
> kernel has just one defconfig whereas ARM32 kernel has
> consolidated and minimized number of defconfigs.
>
> The long term goal for Linux RISC-V is to support single kernel
> image booting on multiple-platforms. Of course, users can always
> strip down the kernel using their custom defconfigs.

OK, thanks for the info.

--
~Randy

2021-09-14 06:33:51

by Heinrich Schuchardt

[permalink] [raw]
Subject: Re: [RFC PATCH V4 6/6] riscv: soc: Add Allwinner SoC kconfig option



On 9/14/21 7:16 AM, Samuel Holland wrote:
> On 9/13/21 10:49 PM, Heinrich Schuchardt wrote:
>> Calling a Kconfig menu item "Allwinner SoCs" which includes all
>> future Allwinner SoCs irritates me. How about CONFIG_SOC_SUNXI_D1
>> instead?
>
> Would you want to have a separate option for each new SoC? That seems
> like the only way to split things up, if you want to be more specific
> than than "sunxi" (or equivalently "sun20i", which is the codename for
> the RISC-V series).
>
> Except at the very beginning (sun4i-sun7i), there have not been clear
> generational boundaries between the various sunxi SoCs, so most of the
> 32-bit ones already get lumped into a single symbol (MACH_SUN8I). And
> there is a single Kconfig symbol, ARCH_SUNXI, for all 64-bit Allwinner SoCs.

On arm64 we have avoided SoC specific Kconfig symbols and left it to the
defconfig to select all SoC specific drivers. Generally the tendency of
the defconfigs is to provide a multiarch kernel. So this should be ok on
RISC-V too.

I was just concerned about the 8250 driver assigned to something called
"Allwinner SoCs" which didn't seem future proof.

>
> There is enough overlap in peripherals that you need a common symbol for
> the peripheral drivers anyway. How about... ARCH_SUNXI? There are 90+
> uses of this symbol throughout drivers/ and sound/, and I have found
> that more than half of them apply to the D1 (see e.g. this commit[1] and
> some of its ancestors).

Looking at the current use of ARCH_SUNXI in drivers reusing the same
name on RISC-V makes sense.

Best regards

Heinrich

>
> RISC-V has so far adopted a CONFIG_SOC_xxx naming scheme, which is
> different from arm/arm64's CONFIG_ARCH_xxx pattern. But now we have a
> case where a SoC family is split between the two architectures. I'm all
> for consistency with the names of other RISC-V platform symbols, but it
> seems that reusing the existing ARCH_SUNXI symbol would be better than
> cluttering up the driver Kconfig files with a duplicate.
>
> Regards,
> Samuel
>
> [1]: https://github.com/smaeul/linux/commit/7841e5c32366
>

2021-09-14 07:21:58

by Maxime Ripard

[permalink] [raw]
Subject: Re: [RFC PATCH V4 6/6] riscv: soc: Add Allwinner SoC kconfig option

Hi,

On Tue, Sep 14, 2021 at 05:49:52AM +0200, Heinrich Schuchardt wrote:
> > Hi,
> >
> > On Sat, Sep 11, 2021 at 05:21:39PM +0800, [email protected] wrote:
> > > From: Liu Shaohua <[email protected]>
> > >
> > > Add Allwinner kconfig option which selects SoC specific and common
> > > drivers that is required for this SoC.
> > >
> > > Allwinner D1 uses custom PTE attributes to solve non-coherency SOC
> > > interconnect issues for dma synchronization, so we set the default
> > > value when SOC_SUNXI selected.
> > >
> > > Signed-off-by: Liu Shaohua <[email protected]>
> > > Signed-off-by: Guo Ren <[email protected]>
> > > Signed-off-by: Wei Fu <[email protected]>
> > > Cc: Anup Patel <[email protected]>
> > > Cc: Atish Patra <[email protected]>
> > > Cc: Christoph Hellwig <[email protected]>
> > > Cc: Chen-Yu Tsai <[email protected]>
> > > Cc: Drew Fustini <[email protected]>
> > > Cc: Maxime Ripard <[email protected]>
> > > Cc: Palmer Dabbelt <[email protected]>
> > > Cc: Wei Wu <[email protected]>
> > > ---
> > > arch/riscv/Kconfig.socs | 15 +++++++++++++++
> > > arch/riscv/configs/defconfig | 1 +
> > > 2 files changed, 16 insertions(+)
> > >
> > > diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
> > > index 30676ebb16eb..8721c000ef23 100644
> > > --- a/arch/riscv/Kconfig.socs
> > > +++ b/arch/riscv/Kconfig.socs
> > > @@ -70,4 +70,19 @@ config SOC_CANAAN_K210_DTB_SOURCE
> > > endif
> > > +config SOC_SUNXI
> > > + bool "Allwinner SoCs"
> > > + depends on MMU
> > > + select DWMAC_GENERIC
> > > + select ERRATA_THEAD
> > > + select RISCV_DMA_NONCOHERENT
> > > + select RISCV_ERRATA_ALTERNATIVE
> > > + select SERIAL_8250
> > > + select SERIAL_8250_CONSOLE
> > > + select SERIAL_8250_DW
> > > + select SIFIVE_PLIC
> > > + select STMMAC_ETH
> > > + help
> > > + This enables support for Allwinner SoC platforms like the D1.
> > > +
> >
> > I'm not sure we should select the drivers there. We could very well
> > imagine a board without UART, or even more so without ethernet.
>
> The draft of the RISC-V platform specification is available here:
> https://github.com/riscv/riscv-platform-specs/blob/main/riscv-platform-spec.adoc#uartserial-console
>
> The specification requires in section "2.1.5.1. UART/Serial Console" that on
> platforms with a rich operating system (e.g. Linux) you have a serial
> console. Hence requiring 8250 support for the D1 CPU is justified.

I mean, not really? The platform is required to have a UART, but nothing
requires the kernel to have a driver for it as far as I'm aware.

Maxime


Attachments:
(No filename) (2.60 kB)
signature.asc (235.00 B)
Download all attachments

2021-09-14 09:32:10

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [RFC PATCH V4 6/6] riscv: soc: Add Allwinner SoC kconfig option

On Tue, Sep 14, 2021 at 4:36 AM Guo Ren <[email protected]> wrote:
> On Tue, Sep 14, 2021 at 2:49 AM Randy Dunlap <[email protected]> wrote:
> > On 9/13/21 2:20 AM, Guo Ren wrote:
> > > On Mon, Sep 13, 2021 at 4:45 PM Maxime Ripard <[email protected]> wrote:
> > >> On Sat, Sep 11, 2021 at 05:21:39PM +0800, [email protected] wrote:
> > >>> From: Liu Shaohua <[email protected]>
> > >>>
> > >>> Add Allwinner kconfig option which selects SoC specific and common
> > >>> drivers that is required for this SoC.
> > >>>
> > >>> Allwinner D1 uses custom PTE attributes to solve non-coherency SOC
> > >>> interconnect issues for dma synchronization, so we set the default
> > >>> value when SOC_SUNXI selected.
> > >>>
> > >>>
> > >>> +config SOC_SUNXI
> > >>> + bool "Allwinner SoCs"
> > >>> + depends on MMU
> > >>> + select DWMAC_GENERIC
> > >>> + select ERRATA_THEAD
> > >>> + select RISCV_DMA_NONCOHERENT
> > >>> + select RISCV_ERRATA_ALTERNATIVE
> > >>> + select SERIAL_8250
> > >>> + select SERIAL_8250_CONSOLE
> > >>> + select SERIAL_8250_DW
> > >>> + select SIFIVE_PLIC
> > >>> + select STMMAC_ETH
> > >>> + help
> > >>> + This enables support for Allwinner SoC platforms like the D1.
> > >>> +
> > >>
> > >> I'm not sure we should select the drivers there. We could very well
> > >> imagine a board without UART, or even more so without ethernet.
> > > We just want people could bring D1 up easier, 8250 is the basic component.
> > >
> > >
> > >>
> > >> These options should be in the defconfig.
> >
> > Agreed, using a defconfig is the right way to do this.
> Put 8250 related configs into arch/riscv/configs/defconfig?

I think that would be best, as well as the STMMAC_ETH and
DWMAC_GENERIC options.

If all RISC-V chips are required to have a 8250 compatible uart,
selecting it from CONFIG_RISCV would work as well, but for
consistency I'd give users the option to leave it out, just like
any other driver that is not required to have a useful system.

> @Palmer Dabbelt @Arnd Bergmann, How do you think about that?
> (defconfig or Kconfig.soc)
> My purpose is when people make the Image from riscv/defconfig, then
> the Image could run on all platforms include D1.

I would try to keep the Kconfig.soc as short as possible. As a general
rule, only use 'select' to enable symbols that are otherwise not user
visible, such as the specific errata if you want to hide them. For individual
SoCs, I prefer not having separate Kconfig options, but instead have
those per driver. We have some SoC families that have part specific
options elsewhere, e.g. drivers/soc/renesas/Kconfig, but I'd only add
those if you can't avoid it. Having it in drivers/soc/ may be better for
sunxi than spreading them over arch/{arm,arm64,riscv}.

Some subsystem maintainers want drivers to be selected by the SoC
option, this is why you need the 'select SIFIVE_PLIC', but usually
the drivers are selectable with a 'depends on ARCH_SUNXI ||
COMPILE_TEST' and enabled in the defconfig.

If you want to get fancy, you can use something like:

config RESET_SUNXI
bool "Allwinner SoCs Reset Driver" if COMPILE_TEST && !ARCH_SUNXI
default ARCH_SUNXI

This will make an option that
- always enabled when the platform is built-in
- user selectable when compile-testing for any other platform
- always disabled otherwise

Arnd

2021-09-14 09:45:20

by Ben Dooks

[permalink] [raw]
Subject: Re: [RFC PATCH V4 6/6] riscv: soc: Add Allwinner SoC kconfig option

On 13/09/2021 09:45, Maxime Ripard wrote:
> Hi,
>
> On Sat, Sep 11, 2021 at 05:21:39PM +0800, [email protected] wrote:
>> From: Liu Shaohua <[email protected]>
>>
>> Add Allwinner kconfig option which selects SoC specific and common
>> drivers that is required for this SoC.
>>
>> Allwinner D1 uses custom PTE attributes to solve non-coherency SOC
>> interconnect issues for dma synchronization, so we set the default
>> value when SOC_SUNXI selected.
>>
>> Signed-off-by: Liu Shaohua <[email protected]>
>> Signed-off-by: Guo Ren <[email protected]>
>> Signed-off-by: Wei Fu <[email protected]>
>> Cc: Anup Patel <[email protected]>
>> Cc: Atish Patra <[email protected]>
>> Cc: Christoph Hellwig <[email protected]>
>> Cc: Chen-Yu Tsai <[email protected]>
>> Cc: Drew Fustini <[email protected]>
>> Cc: Maxime Ripard <[email protected]>
>> Cc: Palmer Dabbelt <[email protected]>
>> Cc: Wei Wu <[email protected]>
>> ---
>> arch/riscv/Kconfig.socs | 15 +++++++++++++++
>> arch/riscv/configs/defconfig | 1 +
>> 2 files changed, 16 insertions(+)
>>
>> diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
>> index 30676ebb16eb..8721c000ef23 100644
>> --- a/arch/riscv/Kconfig.socs
>> +++ b/arch/riscv/Kconfig.socs
>> @@ -70,4 +70,19 @@ config SOC_CANAAN_K210_DTB_SOURCE
>>
>> endif
>>
>> +config SOC_SUNXI
>> + bool "Allwinner SoCs"
>> + depends on MMU
>> + select DWMAC_GENERIC
>> + select ERRATA_THEAD
>> + select RISCV_DMA_NONCOHERENT
>> + select RISCV_ERRATA_ALTERNATIVE
>> + select SERIAL_8250
>> + select SERIAL_8250_CONSOLE
>> + select SERIAL_8250_DW
>> + select SIFIVE_PLIC
>> + select STMMAC_ETH
>> + help
>> + This enables support for Allwinner SoC platforms like the D1.
>> +
>
> I'm not sure we should select the drivers there. We could very well
> imagine a board without UART, or even more so without ethernet.

I could make a case for selecting the serial as it is probably the
console, however the ethernet is not necessary for operation and I
would prefer to see this removed.

I wonder if we should have a new Kconfig keyword of suggest which
marks drivers as suggested for a given SoC/board/platform.

--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html

2021-09-14 10:09:11

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [RFC PATCH V4 6/6] riscv: soc: Add Allwinner SoC kconfig option

On Tue, 14 Sept 2021 at 11:31, Arnd Bergmann <[email protected]> wrote:
> Some subsystem maintainers want drivers to be selected by the SoC
> option, this is why you need the 'select SIFIVE_PLIC', but usually
> the drivers are selectable with a 'depends on ARCH_SUNXI ||
> COMPILE_TEST' and enabled in the defconfig.

I would say selecting drivers is even more useful for distros and
other downstream users. Especially in the ARM world where we have so
many different SoCs - how could a distro person know which driver is
necessary, important or useful? Having all main SoC drivers enabled
when ARCH_SUNXI=y, helps distro a lot.

> If you want to get fancy, you can use something like:
>
> config RESET_SUNXI
> bool "Allwinner SoCs Reset Driver" if COMPILE_TEST && !ARCH_SUNXI
> default ARCH_SUNXI
>
> This will make an option that
> - always enabled when the platform is built-in
> - user selectable when compile-testing for any other platform
> - always disabled otherwise

+1 for this pattern.

Best regards,
Krzysztof

2021-09-14 10:15:21

by Maxime Ripard

[permalink] [raw]
Subject: Re: [RFC PATCH V4 6/6] riscv: soc: Add Allwinner SoC kconfig option

On Tue, Sep 14, 2021 at 12:07:08PM +0200, Krzysztof Kozlowski wrote:
> On Tue, 14 Sept 2021 at 11:31, Arnd Bergmann <[email protected]> wrote:
> > Some subsystem maintainers want drivers to be selected by the SoC
> > option, this is why you need the 'select SIFIVE_PLIC', but usually
> > the drivers are selectable with a 'depends on ARCH_SUNXI ||
> > COMPILE_TEST' and enabled in the defconfig.
>
> I would say selecting drivers is even more useful for distros and
> other downstream users. Especially in the ARM world where we have so
> many different SoCs - how could a distro person know which driver is
> necessary, important or useful? Having all main SoC drivers enabled
> when ARCH_SUNXI=y, helps distro a lot.

Imply, maybe, but select is far too painful for everyone else.

> > If you want to get fancy, you can use something like:
> >
> > config RESET_SUNXI
> > bool "Allwinner SoCs Reset Driver" if COMPILE_TEST && !ARCH_SUNXI
> > default ARCH_SUNXI
> >
> > This will make an option that
> > - always enabled when the platform is built-in
> > - user selectable when compile-testing for any other platform
> > - always disabled otherwise
>
> +1 for this pattern.

Yeah, or a default

Maxime


Attachments:
(No filename) (1.22 kB)
signature.asc (235.00 B)
Download all attachments

2021-09-14 12:11:58

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [RFC PATCH V4 6/6] riscv: soc: Add Allwinner SoC kconfig option

On 14/09/2021 12:13, Maxime Ripard wrote:
> On Tue, Sep 14, 2021 at 12:07:08PM +0200, Krzysztof Kozlowski wrote:
>> On Tue, 14 Sept 2021 at 11:31, Arnd Bergmann <[email protected]> wrote:
>>> Some subsystem maintainers want drivers to be selected by the SoC
>>> option, this is why you need the 'select SIFIVE_PLIC', but usually
>>> the drivers are selectable with a 'depends on ARCH_SUNXI ||
>>> COMPILE_TEST' and enabled in the defconfig.
>>
>> I would say selecting drivers is even more useful for distros and
>> other downstream users. Especially in the ARM world where we have so
>> many different SoCs - how could a distro person know which driver is
>> necessary, important or useful? Having all main SoC drivers enabled
>> when ARCH_SUNXI=y, helps distro a lot.
>
> Imply, maybe, but select is far too painful for everyone else.

If we talk about UART driver, then sure - imply makes sense. But if we
talk about core SoC components necessary for boot (e.g. timers, clocks,
pinctrl), then select is appropriate. There is no point to enable
ARCH_XXX without these core components.


Best regards,
Krzysztof

2021-09-14 13:09:07

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [RFC PATCH V4 6/6] riscv: soc: Add Allwinner SoC kconfig option

On Tue, Sep 14, 2021 at 2:11 PM Krzysztof Kozlowski <[email protected]> wrote:
>
> On 14/09/2021 12:13, Maxime Ripard wrote:
> > On Tue, Sep 14, 2021 at 12:07:08PM +0200, Krzysztof Kozlowski wrote:
> >> On Tue, 14 Sept 2021 at 11:31, Arnd Bergmann <[email protected]> wrote:
> >>> Some subsystem maintainers want drivers to be selected by the SoC
> >>> option, this is why you need the 'select SIFIVE_PLIC', but usually
> >>> the drivers are selectable with a 'depends on ARCH_SUNXI ||
> >>> COMPILE_TEST' and enabled in the defconfig.
> >>
> >> I would say selecting drivers is even more useful for distros and
> >> other downstream users. Especially in the ARM world where we have so
> >> many different SoCs - how could a distro person know which driver is
> >> necessary, important or useful? Having all main SoC drivers enabled
> >> when ARCH_SUNXI=y, helps distro a lot.
> >
> > Imply, maybe, but select is far too painful for everyone else.
>
> If we talk about UART driver, then sure - imply makes sense. But if we
> talk about core SoC components necessary for boot (e.g. timers, clocks,
> pinctrl), then select is appropriate. There is no point to enable
> ARCH_XXX without these core components.

Please never use 'imply', this is functionally the same as 'default', just
the wrong way around, like the infamous 'comefrom' instruction
in programming languages ;-)

I still prefer using defaults and defconfig files over 'select', but I can
see the use of select in some cases, as long as the symbol you
are selecting is not already user visible.

Arnd

2021-09-15 07:49:00

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [RFC PATCH V4 2/6] riscv: errata: pgtable: Add custom Svpbmt supported for Allwinner D1

This is the wrong way around. We need to design around the PBMT
definitions. If we want to hack in support for completely broken SOCs
that intentionally violate the specification it should be done after
the fact, in a separate patch, using alternatives and clearly documenting
how broken these SOCs are.

2021-09-15 07:50:26

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [RFC PATCH V4 3/6] RISC-V: Support a new config option for non-coherent DMA

On Sat, Sep 11, 2021 at 05:21:36PM +0800, [email protected] wrote:
> + select DMA_GLOBAL_POOL
> + select DMA_DIRECT_REMAP

No need to select DMA_GLOBAL_POOL when DMA_DIRECT_REMAP is select.

Also a patch just to add a option that is not selected and won't build
if selected does not make sense.

2021-09-15 07:51:57

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [RFC PATCH V4 4/6] RISC-V: Implement arch_sync_dma* functions

On Sat, Sep 11, 2021 at 05:21:37PM +0800, [email protected] wrote:
> +static void __dma_sync(phys_addr_t paddr, size_t size, enum dma_data_direction dir)
> +{
> + if ((dir == DMA_FROM_DEVICE) && (dma_cache_sync->cache_invalidate))
> + dma_cache_sync->cache_invalidate(paddr, size);
> + else if ((dir == DMA_TO_DEVICE) && (dma_cache_sync->cache_clean))
> + dma_cache_sync->cache_clean(paddr, size);
> + else if ((dir == DMA_BIDIRECTIONAL) && dma_cache_sync->cache_flush)
> + dma_cache_sync->cache_flush(paddr, size);
> +}

Despite various snipplets this is a still pretty much the broken previous
versions. These need to use the CMO instructions directly which are
about to go into review, and then your SBI can trap on those can call
whatever non-standard mess you're using.

2021-09-16 00:59:08

by Guo Ren

[permalink] [raw]
Subject: Re: [RFC PATCH V4 2/6] riscv: errata: pgtable: Add custom Svpbmt supported for Allwinner D1

On Wed, Sep 15, 2021 at 3:47 PM Christoph Hellwig <[email protected]> wrote:
>
> This is the wrong way around. We need to design around the PBMT
> definitions.
I've defined them in arch/riscv/include/asm/pgtable-bits.h first, that
follow current latest draft. Then I undefine them in
errata/thead/errata.c to illustrate we should follow standard PBMT,
not a custom one.

> If we want to hack in support for completely broken SOCs
> that intentionally violate the specification it should be done after
> the fact,
When c9xx was released in 2018, there is no clear direction for how to
solve the non-coherent problem. We just thought PBMT is the correct
direction, but we can't predict how encoding exactly in the highest
bits. (Maybe we should keep the highest bits zero for _P/SXXX in
pgtable.h, but it was really hard to guess at that time.) So don't
imply we "intentionally" here! When the svpbmt is frozen in the
future, we would follow that in our next-generation processor.

> in a separate patch, using alternatives and clearly documenting
> how broken these SOCs are.
Okay, I would separate errata into another patch.

About documenting, I've illustrated c9xx PTE format's detail and using
undef _PAGE_XXX to show that we replaced standard's in errata:

+/*
+ * T-HEAD C9xx PTE format:
+ * | 63 | 62 | 61 | 60 | 59-8 | 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0
+ * SO C B SH RSW D A G U X W R V
+ * ^ ^ ^ ^ ^
+ * BIT(63): SO - Strong Order
+ * BIT(62): C - Cacheable
+ * BIT(61): B - Bufferable
+ * BIT(60): SH - Shareable
+ *
+ * MT_MASK : [63 - 59]
+ * MT_PMA : C + SH
+ * MT_NC : (none)
+ * MT_IO : SO
+ */
+#undef _PAGE_MT_MASK
+#undef _PAGE_MT_PMA
+#undef _PAGE_MT_NC
+#undef _PAGE_MT_IO
+
+#define _PAGE_MT_MASK 0xf800000000000000
+#define _PAGE_MT_PMA 0x5000000000000000
+#define _PAGE_MT_NC 0x0
+#define _PAGE_MT_IO 0x8000000000000000
+#endif


--
Best Regards
Guo Ren

ML: https://lore.kernel.org/linux-csky/

2021-09-16 01:21:38

by Guo Ren

[permalink] [raw]
Subject: Re: [RFC PATCH V4 3/6] RISC-V: Support a new config option for non-coherent DMA

On Wed, Sep 15, 2021 at 3:48 PM Christoph Hellwig <[email protected]> wrote:
>
> On Sat, Sep 11, 2021 at 05:21:36PM +0800, [email protected] wrote:
> > + select DMA_GLOBAL_POOL
> > + select DMA_DIRECT_REMAP
>
> No need to select DMA_GLOBAL_POOL when DMA_DIRECT_REMAP is select.
If we want to support PBMT & global_dma_pool both in riscv. Could they
work together in arch/riscv with [1]?

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

>
> Also a patch just to add a option that is not selected and won't build
> if selected does not make sense.
I just want to rebase on Atish's patch and append DMA_DIRECT_REMAP.
Okay, DMA_DIRECT_REMAP & DMA_GLOBAL_POOL should be separated from the
patch.



--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -156,9 +156,14 @@ void *dma_direct_alloc(struct device *dev, size_t size,

if (!IS_ENABLED(CONFIG_ARCH_HAS_DMA_SET_UNCACHED) &&
!IS_ENABLED(CONFIG_DMA_DIRECT_REMAP) &&
+ !IS_ENABLED(CONFIG_DMA_GLOBAL_POOL) &&
!dev_is_dma_coherent(dev))
return arch_dma_alloc(dev, size, dma_handle, gfp, attrs);

+ if (IS_ENABLED(CONFIG_DMA_GLOBAL_POOL) &&
+ !dev_is_dma_coherent(dev))
+ return dma_alloc_from_global_coherent(dev, size, dma_handle);
+
/*
* Remapping or decrypting memory may block. If either is required and
* we can't block, allocate the memory from the atomic pools.
@@ -255,11 +260,19 @@ void dma_direct_free(struct device *dev, size_t size,

if (!IS_ENABLED(CONFIG_ARCH_HAS_DMA_SET_UNCACHED) &&
!IS_ENABLED(CONFIG_DMA_DIRECT_REMAP) &&
+ !IS_ENABLED(CONFIG_DMA_GLOBAL_POOL) &&
!dev_is_dma_coherent(dev)) {
arch_dma_free(dev, size, cpu_addr, dma_addr, attrs);
return;
}

+ if (IS_ENABLED(CONFIG_DMA_GLOBAL_POOL) &&
+ !dev_is_dma_coherent(dev)) {
+ if (!dma_release_from_global_coherent(page_order, cpu_addr))
+ WARN_ON_ONCE(1);
+ return;
+ }
+
Here CONFIG_DMA_GLOBAL_POOL is independent from CONFIG_DMA_DIRECT_REMAP.

/* If cpu_addr is not from an atomic pool, dma_free_from_pool() fails */
if (IS_ENABLED(CONFIG_DMA_COHERENT_POOL) &&
dma_free_from_pool(dev, cpu_addr, PAGE_ALIGN(size)))

--
Best Regards
Guo Ren

ML: https://lore.kernel.org/linux-csky/

2021-09-16 01:35:29

by Guo Ren

[permalink] [raw]
Subject: Re: [RFC PATCH V4 4/6] RISC-V: Implement arch_sync_dma* functions

On Wed, Sep 15, 2021 at 3:50 PM Christoph Hellwig <[email protected]> wrote:
>
> On Sat, Sep 11, 2021 at 05:21:37PM +0800, [email protected] wrote:
> > +static void __dma_sync(phys_addr_t paddr, size_t size, enum dma_data_direction dir)
> > +{
> > + if ((dir == DMA_FROM_DEVICE) && (dma_cache_sync->cache_invalidate))
> > + dma_cache_sync->cache_invalidate(paddr, size);
> > + else if ((dir == DMA_TO_DEVICE) && (dma_cache_sync->cache_clean))
> > + dma_cache_sync->cache_clean(paddr, size);
> > + else if ((dir == DMA_BIDIRECTIONAL) && dma_cache_sync->cache_flush)
> > + dma_cache_sync->cache_flush(paddr, size);
> > +}
>
> Despite various snipplets this is a still pretty much the broken previous
> versions. These need to use the CMO instructions directly which are
> about to go into review, and then your SBI can trap on those can call
> whatever non-standard mess you're using.
I think you mean put an ALTERNATIVE slot in the prologue of __dma_sync?

#define ALT_DMA_SYNC() \
asm(ALTERNATIVE(".rept 64\n nop\n .endr\n", "<vendor code>",
XXX_VENDOR_ID, \
ERRATA_XXX, CONFIG_ERRATA_XXX) \
: : : "memory")

static void __dma_sync(phys_addr_t paddr, size_t size, enum
dma_data_direction dir)
{
ALT_DMA_SYNC();

/* future cmo codes */
}




--
Best Regards
Guo Ren

ML: https://lore.kernel.org/linux-csky/

2021-09-16 04:25:57

by Anup Patel

[permalink] [raw]
Subject: Re: [RFC PATCH V4 4/6] RISC-V: Implement arch_sync_dma* functions

On Thu, Sep 16, 2021 at 7:03 AM Guo Ren <[email protected]> wrote:
>
> On Wed, Sep 15, 2021 at 3:50 PM Christoph Hellwig <[email protected]> wrote:
> >
> > On Sat, Sep 11, 2021 at 05:21:37PM +0800, [email protected] wrote:
> > > +static void __dma_sync(phys_addr_t paddr, size_t size, enum dma_data_direction dir)
> > > +{
> > > + if ((dir == DMA_FROM_DEVICE) && (dma_cache_sync->cache_invalidate))
> > > + dma_cache_sync->cache_invalidate(paddr, size);
> > > + else if ((dir == DMA_TO_DEVICE) && (dma_cache_sync->cache_clean))
> > > + dma_cache_sync->cache_clean(paddr, size);
> > > + else if ((dir == DMA_BIDIRECTIONAL) && dma_cache_sync->cache_flush)
> > > + dma_cache_sync->cache_flush(paddr, size);
> > > +}
> >
> > Despite various snipplets this is a still pretty much the broken previous
> > versions. These need to use the CMO instructions directly which are
> > about to go into review, and then your SBI can trap on those can call
> > whatever non-standard mess you're using.
> I think you mean put an ALTERNATIVE slot in the prologue of __dma_sync?
>
> #define ALT_DMA_SYNC() \
> asm(ALTERNATIVE(".rept 64\n nop\n .endr\n", "<vendor code>",
> XXX_VENDOR_ID, \
> ERRATA_XXX, CONFIG_ERRATA_XXX) \
> : : : "memory")
>
> static void __dma_sync(phys_addr_t paddr, size_t size, enum
> dma_data_direction dir)
> {
> ALT_DMA_SYNC();
>
> /* future cmo codes */
> }

I think Christoph is suggesting to always use CMO instructions for
implementing arch specific DMA sync functions. The SBI implementation
will trap-n-emulate CMO instructions when underlying HW does not
have it. This means custom cache instructions on D1 can reside in
the platform support code of OpenSBI.

I also agree with the above suggestion. At least, this will ensure that we
have only one way of doing cache operations from S-mode perspective
which is CMO instructions.

Regards,
Anup

>
>
>
>
> --
> Best Regards
> Guo Ren
>
> ML: https://lore.kernel.org/linux-csky/

2021-09-16 04:41:24

by Atish Patra

[permalink] [raw]
Subject: Re: [RFC PATCH V4 3/6] RISC-V: Support a new config option for non-coherent DMA

On Wed, Sep 15, 2021 at 6:21 PM Guo Ren <[email protected]> wrote:
>
> On Wed, Sep 15, 2021 at 3:48 PM Christoph Hellwig <[email protected]> wrote:
> >
> > On Sat, Sep 11, 2021 at 05:21:36PM +0800, [email protected] wrote:
> > > + select DMA_GLOBAL_POOL
> > > + select DMA_DIRECT_REMAP
> >
> > No need to select DMA_GLOBAL_POOL when DMA_DIRECT_REMAP is select.
> If we want to support PBMT & global_dma_pool both in riscv. Could they
> work together in arch/riscv with [1]?

We don't have to worry about it as the next version of my series will
use the simpler
dma_uncached functionality to support uncached window approach taken
by starlight socs.

I was supposed to send it sooner but got busy with PMU stuff and a bad flu :(.

>
> [1]: https://lore.kernel.org/lkml/[email protected]/T/
>
> >
> > Also a patch just to add a option that is not selected and won't build
> > if selected does not make sense.
> I just want to rebase on Atish's patch and append DMA_DIRECT_REMAP.
> Okay, DMA_DIRECT_REMAP & DMA_GLOBAL_POOL should be separated from the
> patch.
>
>
>
> --- a/kernel/dma/direct.c
> +++ b/kernel/dma/direct.c
> @@ -156,9 +156,14 @@ void *dma_direct_alloc(struct device *dev, size_t size,
>
> if (!IS_ENABLED(CONFIG_ARCH_HAS_DMA_SET_UNCACHED) &&
> !IS_ENABLED(CONFIG_DMA_DIRECT_REMAP) &&
> + !IS_ENABLED(CONFIG_DMA_GLOBAL_POOL) &&
> !dev_is_dma_coherent(dev))
> return arch_dma_alloc(dev, size, dma_handle, gfp, attrs);
>
> + if (IS_ENABLED(CONFIG_DMA_GLOBAL_POOL) &&
> + !dev_is_dma_coherent(dev))
> + return dma_alloc_from_global_coherent(dev, size, dma_handle);
> +
> /*
> * Remapping or decrypting memory may block. If either is required and
> * we can't block, allocate the memory from the atomic pools.
> @@ -255,11 +260,19 @@ void dma_direct_free(struct device *dev, size_t size,
>
> if (!IS_ENABLED(CONFIG_ARCH_HAS_DMA_SET_UNCACHED) &&
> !IS_ENABLED(CONFIG_DMA_DIRECT_REMAP) &&
> + !IS_ENABLED(CONFIG_DMA_GLOBAL_POOL) &&
> !dev_is_dma_coherent(dev)) {
> arch_dma_free(dev, size, cpu_addr, dma_addr, attrs);
> return;
> }
>
> + if (IS_ENABLED(CONFIG_DMA_GLOBAL_POOL) &&
> + !dev_is_dma_coherent(dev)) {
> + if (!dma_release_from_global_coherent(page_order, cpu_addr))
> + WARN_ON_ONCE(1);
> + return;
> + }
> +
> Here CONFIG_DMA_GLOBAL_POOL is independent from CONFIG_DMA_DIRECT_REMAP.
>
> /* If cpu_addr is not from an atomic pool, dma_free_from_pool() fails */
> if (IS_ENABLED(CONFIG_DMA_COHERENT_POOL) &&
> dma_free_from_pool(dev, cpu_addr, PAGE_ALIGN(size)))
>
> --
> Best Regards
> Guo Ren
>
> ML: https://lore.kernel.org/linux-csky/
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv



--
Regards,
Atish

2021-09-16 04:46:33

by Atish Patra

[permalink] [raw]
Subject: Re: [RFC PATCH V4 4/6] RISC-V: Implement arch_sync_dma* functions

On Wed, Sep 15, 2021 at 9:25 PM Anup Patel <[email protected]> wrote:
>
> On Thu, Sep 16, 2021 at 7:03 AM Guo Ren <[email protected]> wrote:
> >
> > On Wed, Sep 15, 2021 at 3:50 PM Christoph Hellwig <[email protected]> wrote:
> > >
> > > On Sat, Sep 11, 2021 at 05:21:37PM +0800, [email protected] wrote:
> > > > +static void __dma_sync(phys_addr_t paddr, size_t size, enum dma_data_direction dir)
> > > > +{
> > > > + if ((dir == DMA_FROM_DEVICE) && (dma_cache_sync->cache_invalidate))
> > > > + dma_cache_sync->cache_invalidate(paddr, size);
> > > > + else if ((dir == DMA_TO_DEVICE) && (dma_cache_sync->cache_clean))
> > > > + dma_cache_sync->cache_clean(paddr, size);
> > > > + else if ((dir == DMA_BIDIRECTIONAL) && dma_cache_sync->cache_flush)
> > > > + dma_cache_sync->cache_flush(paddr, size);
> > > > +}
> > >
> > > Despite various snipplets this is a still pretty much the broken previous
> > > versions. These need to use the CMO instructions directly which are
> > > about to go into review, and then your SBI can trap on those can call
> > > whatever non-standard mess you're using.
> > I think you mean put an ALTERNATIVE slot in the prologue of __dma_sync?
> >
> > #define ALT_DMA_SYNC() \
> > asm(ALTERNATIVE(".rept 64\n nop\n .endr\n", "<vendor code>",
> > XXX_VENDOR_ID, \
> > ERRATA_XXX, CONFIG_ERRATA_XXX) \
> > : : : "memory")
> >
> > static void __dma_sync(phys_addr_t paddr, size_t size, enum
> > dma_data_direction dir)
> > {
> > ALT_DMA_SYNC();
> >
> > /* future cmo codes */
> > }
>
> I think Christoph is suggesting to always use CMO instructions for
> implementing arch specific DMA sync functions. The SBI implementation
> will trap-n-emulate CMO instructions when underlying HW does not
> have it. This means custom cache instructions on D1 can reside in
> the platform support code of OpenSBI.
>
> I also agree with the above suggestion. At least, this will ensure that we
> have only one way of doing cache operations from S-mode perspective
> which is CMO instructions.
>

Sounds good to me. For stralight Socs, we may need to add a l2 cache controller
driver in OpenSBI as well. IIRC, sifive cores do have some M-mode only
CMO instructions
as well but they may not align with CMO spec. I have to double check.


> Regards,
> Anup
>
> >
> >
> >
> >
> > --
> > Best Regards
> > Guo Ren
> >
> > ML: https://lore.kernel.org/linux-csky/
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv



--
Regards,
Atish

2021-09-16 06:10:45

by Guo Ren

[permalink] [raw]
Subject: Re: [RFC PATCH V4 3/6] RISC-V: Support a new config option for non-coherent DMA

On Thu, Sep 16, 2021 at 12:39 PM Atish Patra <[email protected]> wrote:
>
> On Wed, Sep 15, 2021 at 6:21 PM Guo Ren <[email protected]> wrote:
> >
> > On Wed, Sep 15, 2021 at 3:48 PM Christoph Hellwig <[email protected]> wrote:
> > >
> > > On Sat, Sep 11, 2021 at 05:21:36PM +0800, [email protected] wrote:
> > > > + select DMA_GLOBAL_POOL
> > > > + select DMA_DIRECT_REMAP
> > >
> > > No need to select DMA_GLOBAL_POOL when DMA_DIRECT_REMAP is select.
> > If we want to support PBMT & global_dma_pool both in riscv. Could they
> > work together in arch/riscv with [1]?
>
> We don't have to worry about it as the next version of my series will
> use the simpler
> dma_uncached functionality to support uncached window approach taken
> by starlight socs.
Sounds good, thx.

>
> I was supposed to send it sooner but got busy with PMU stuff and a bad flu :(.
Take good care of yourself and hope you feel better soon.

>
> >
> > [1]: https://lore.kernel.org/lkml/[email protected]/T/
> >
> > >
> > > Also a patch just to add a option that is not selected and won't build
> > > if selected does not make sense.
> > I just want to rebase on Atish's patch and append DMA_DIRECT_REMAP.
> > Okay, DMA_DIRECT_REMAP & DMA_GLOBAL_POOL should be separated from the
> > patch.
> >
> >
> >
> > --- a/kernel/dma/direct.c
> > +++ b/kernel/dma/direct.c
> > @@ -156,9 +156,14 @@ void *dma_direct_alloc(struct device *dev, size_t size,
> >
> > if (!IS_ENABLED(CONFIG_ARCH_HAS_DMA_SET_UNCACHED) &&
> > !IS_ENABLED(CONFIG_DMA_DIRECT_REMAP) &&
> > + !IS_ENABLED(CONFIG_DMA_GLOBAL_POOL) &&
> > !dev_is_dma_coherent(dev))
> > return arch_dma_alloc(dev, size, dma_handle, gfp, attrs);
> >
> > + if (IS_ENABLED(CONFIG_DMA_GLOBAL_POOL) &&
> > + !dev_is_dma_coherent(dev))
> > + return dma_alloc_from_global_coherent(dev, size, dma_handle);
> > +
> > /*
> > * Remapping or decrypting memory may block. If either is required and
> > * we can't block, allocate the memory from the atomic pools.
> > @@ -255,11 +260,19 @@ void dma_direct_free(struct device *dev, size_t size,
> >
> > if (!IS_ENABLED(CONFIG_ARCH_HAS_DMA_SET_UNCACHED) &&
> > !IS_ENABLED(CONFIG_DMA_DIRECT_REMAP) &&
> > + !IS_ENABLED(CONFIG_DMA_GLOBAL_POOL) &&
> > !dev_is_dma_coherent(dev)) {
> > arch_dma_free(dev, size, cpu_addr, dma_addr, attrs);
> > return;
> > }
> >
> > + if (IS_ENABLED(CONFIG_DMA_GLOBAL_POOL) &&
> > + !dev_is_dma_coherent(dev)) {
> > + if (!dma_release_from_global_coherent(page_order, cpu_addr))
> > + WARN_ON_ONCE(1);
> > + return;
> > + }
> > +
> > Here CONFIG_DMA_GLOBAL_POOL is independent from CONFIG_DMA_DIRECT_REMAP.
> >
> > /* If cpu_addr is not from an atomic pool, dma_free_from_pool() fails */
> > if (IS_ENABLED(CONFIG_DMA_COHERENT_POOL) &&
> > dma_free_from_pool(dev, cpu_addr, PAGE_ALIGN(size)))
> >
> > --
> > Best Regards
> > Guo Ren
> >
> > ML: https://lore.kernel.org/linux-csky/
> >
> > _______________________________________________
> > linux-riscv mailing list
> > [email protected]
> > http://lists.infradead.org/mailman/listinfo/linux-riscv
>
>
>
> --
> Regards,
> Atish



--
Best Regards
Guo Ren

ML: https://lore.kernel.org/linux-csky/

2021-09-16 06:39:03

by Guo Ren

[permalink] [raw]
Subject: Re: [RFC PATCH V4 6/6] riscv: soc: Add Allwinner SoC kconfig option

On Tue, Sep 14, 2021 at 5:30 PM Arnd Bergmann <[email protected]> wrote:
>
> On Tue, Sep 14, 2021 at 4:36 AM Guo Ren <[email protected]> wrote:
> > On Tue, Sep 14, 2021 at 2:49 AM Randy Dunlap <[email protected]> wrote:
> > > On 9/13/21 2:20 AM, Guo Ren wrote:
> > > > On Mon, Sep 13, 2021 at 4:45 PM Maxime Ripard <[email protected]> wrote:
> > > >> On Sat, Sep 11, 2021 at 05:21:39PM +0800, [email protected] wrote:
> > > >>> From: Liu Shaohua <[email protected]>
> > > >>>
> > > >>> Add Allwinner kconfig option which selects SoC specific and common
> > > >>> drivers that is required for this SoC.
> > > >>>
> > > >>> Allwinner D1 uses custom PTE attributes to solve non-coherency SOC
> > > >>> interconnect issues for dma synchronization, so we set the default
> > > >>> value when SOC_SUNXI selected.
> > > >>>
> > > >>>
> > > >>> +config SOC_SUNXI
> > > >>> + bool "Allwinner SoCs"
> > > >>> + depends on MMU
> > > >>> + select DWMAC_GENERIC
> > > >>> + select ERRATA_THEAD
> > > >>> + select RISCV_DMA_NONCOHERENT
> > > >>> + select RISCV_ERRATA_ALTERNATIVE
> > > >>> + select SERIAL_8250
> > > >>> + select SERIAL_8250_CONSOLE
> > > >>> + select SERIAL_8250_DW
> > > >>> + select SIFIVE_PLIC
> > > >>> + select STMMAC_ETH
> > > >>> + help
> > > >>> + This enables support for Allwinner SoC platforms like the D1.
> > > >>> +
> > > >>
> > > >> I'm not sure we should select the drivers there. We could very well
> > > >> imagine a board without UART, or even more so without ethernet.
> > > > We just want people could bring D1 up easier, 8250 is the basic component.
> > > >
> > > >
> > > >>
> > > >> These options should be in the defconfig.
> > >
> > > Agreed, using a defconfig is the right way to do this.
> > Put 8250 related configs into arch/riscv/configs/defconfig?
>
> I think that would be best, as well as the STMMAC_ETH and
> DWMAC_GENERIC options.
Okay, I would move STMMAC_ETH SERIAL_8250_DW SERIAL_8250_CONSOLE
SERIAL_8250 & SIFIVE_PLIC into defconfig in the next version of the
patch.

>
> If all RISC-V chips are required to have a 8250 compatible uart,
> selecting it from CONFIG_RISCV would work as well, but for
> consistency I'd give users the option to leave it out, just like
> any other driver that is not required to have a useful system.
>
> > @Palmer Dabbelt @Arnd Bergmann, How do you think about that?
> > (defconfig or Kconfig.soc)
> > My purpose is when people make the Image from riscv/defconfig, then
> > the Image could run on all platforms include D1.
>
> I would try to keep the Kconfig.soc as short as possible. As a general
> rule, only use 'select' to enable symbols that are otherwise not user
> visible, such as the specific errata if you want to hide them. For individual
> SoCs, I prefer not having separate Kconfig options, but instead have
> those per driver. We have some SoC families that have part specific
> options elsewhere, e.g. drivers/soc/renesas/Kconfig, but I'd only add
> those if you can't avoid it. Having it in drivers/soc/ may be better for
> sunxi than spreading them over arch/{arm,arm64,riscv}.
>
> Some subsystem maintainers want drivers to be selected by the SoC
> option, this is why you need the 'select SIFIVE_PLIC', but usually
> the drivers are selectable with a 'depends on ARCH_SUNXI ||
> COMPILE_TEST' and enabled in the defconfig.
>
> If you want to get fancy, you can use something like:
>
> config RESET_SUNXI
> bool "Allwinner SoCs Reset Driver" if COMPILE_TEST && !ARCH_SUNXI
> default ARCH_SUNXI
>
> This will make an option that
> - always enabled when the platform is built-in
> - user selectable when compile-testing for any other platform
> - always disabled otherwise
>
> Arnd



--
Best Regards
Guo Ren

ML: https://lore.kernel.org/linux-csky/

2021-09-16 07:33:37

by Atish Patra

[permalink] [raw]
Subject: Re: [RFC PATCH V4 2/6] riscv: errata: pgtable: Add custom Svpbmt supported for Allwinner D1

On Sat, Sep 11, 2021 at 2:23 AM <[email protected]> wrote:
>
> From: Guo Ren <[email protected]>
>
> RISC-V Svpbmt is gradually maturing, the draft is:
>
> Svpbmt PTE format:
> | 63 | 62-61 | 60-8 | 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0
> N MT RSW D A G U X W R V
> ^
>
> Of the Reserved bits [63:54] in a leaf PTE, the high bit is already
> allocated (as the N bit), so bits [62:61] are used as the MT (aka
> MemType) field. This field specifies one of three memory types that
> are close equivalents (or equivalent in effect) to the three main x86
> and ARMv8 memory types - as shown in the following table.
>
> RISC-V
> Encoding &
> MemType RISC-V Description
> ---------- ------------------------------------------------
> 00 - PMA Normal Cacheable, No change to implied PMA memory type
> 01 - NC Non-cacheable, idempotent, weakly-ordered Main Memory
> 10 - IO Non-cacheable, non-idempotent, strongly-ordered I/O memory
> 11 - Rsvd Reserved for future standard use
>
> But T-HEAD C906 in Allwinner D1 has defined a custom Svpbmt:
>
> T-HEAD C9xx PTE format:
> | 63 | 62 | 61 | 60 | 59-8 | 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0
> SO C B SH RSW D A G U X W R V
> ^ ^ ^ ^
> BIT(63): SO - Strong Order
> BIT(62): C - Cacheable
> BIT(61): B - Bufferable
> BIT(60): SH - Shareable
>
> MT_MASK : [63 - 59]
> MT_PMA : C + SH
> MT_NC : (none)
> MT_IO : SO
>
> The patch not only implements the D1's PBMT extension but also
> considers future scalability by errata framework.
>
> We are trying to keep both below work together:
> - "riscv spec acceptance policy" (Svpbmt extension in future)
> - "Linux Keep real hardware work" (Allwinner D1's custom Svpbmt)
>
> Signed-off-by: Guo Ren <[email protected]>
> Signed-off-by: Liu Shaohua <[email protected]>
> Signed-off-by: Wei Fu <[email protected]>
> Cc: Palmer Dabbelt <[email protected]>
> Cc: Christoph Hellwig <[email protected]>
> Cc: Anup Patel <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Cc: Drew Fustini <[email protected]>
> Cc: Wei Fu <[email protected]>
> Cc: Wei Wu <[email protected]>
> Cc: Chen-Yu Tsai <[email protected]>
> Cc: Maxime Ripard <[email protected]>
> Cc: Daniel Lustig <[email protected]>
> Cc: Greg Favor <[email protected]>
> Cc: Andrea Mondelli <[email protected]>
> Cc: Jonathan Behrens <[email protected]>
> Cc: Xinhaoqu (Freddie) <[email protected]>
> Cc: Bill Huffman <[email protected]>
> Cc: Nick Kossifidis <[email protected]>
> Cc: Allen Baum <[email protected]>
> Cc: Josh Scheid <[email protected]>
> Cc: Richard Trauben <[email protected]>
> ---
> arch/riscv/Kconfig.erratas | 11 ++++++
> arch/riscv/errata/Makefile | 1 +
> arch/riscv/errata/alternative.c | 18 ++++++++++
> arch/riscv/errata/thead/Makefile | 1 +
> arch/riscv/errata/thead/errata.c | 47 ++++++++++++++++++++++++++
> arch/riscv/include/asm/alternative.h | 2 ++
> arch/riscv/include/asm/fixmap.h | 2 +-
> arch/riscv/include/asm/pgtable-64.h | 8 +++--
> arch/riscv/include/asm/pgtable-bits.h | 46 +++++++++++++++++++++++--
> arch/riscv/include/asm/pgtable.h | 30 ++++++++++------
> arch/riscv/include/asm/vendorid_list.h | 1 +
> arch/riscv/mm/init.c | 6 ++++
> 12 files changed, 157 insertions(+), 16 deletions(-)
> create mode 100644 arch/riscv/errata/thead/Makefile
> create mode 100644 arch/riscv/errata/thead/errata.c
>
> diff --git a/arch/riscv/Kconfig.erratas b/arch/riscv/Kconfig.erratas
> index b44d6ecdb46e..e130fd040494 100644
> --- a/arch/riscv/Kconfig.erratas
> +++ b/arch/riscv/Kconfig.erratas
> @@ -41,4 +41,15 @@ config ERRATA_SIFIVE_CIP_1200
>
> If you don't know what to do here, say "Y".
>
> +config ERRATA_THEAD
> + bool "T-HEAD errata"
> + depends on RISCV_ERRATA_ALTERNATIVE
> + default y
> + help
> + All T-HEAD errata Kconfig depend on this Kconfig. Disabling
> + this Kconfig will disable all T-HEAD errata. Please say "Y"
> + here if your platform uses T-HEAD CPU cores.
> +
> + Otherwise, please say "N" here to avoid unnecessary overhead.
> +
> endmenu
> diff --git a/arch/riscv/errata/Makefile b/arch/riscv/errata/Makefile
> index b8f8740a3e44..f6db15055e73 100644
> --- a/arch/riscv/errata/Makefile
> +++ b/arch/riscv/errata/Makefile
> @@ -1,2 +1,3 @@
> obj-y += alternative.o
> obj-$(CONFIG_ERRATA_SIFIVE) += sifive/
> +obj-$(CONFIG_ERRATA_THEAD) += thead/
> diff --git a/arch/riscv/errata/alternative.c b/arch/riscv/errata/alternative.c
> index 3b15885db70b..b879aa546bc5 100644
> --- a/arch/riscv/errata/alternative.c
> +++ b/arch/riscv/errata/alternative.c
> @@ -72,3 +72,21 @@ void __init apply_boot_alternatives(void)
> cpu_mfr_info.arch_id, cpu_mfr_info.imp_id);
> }
>
> +/*
> + * This is called very early form setup_vm in the boot process.
> + */
> +void __init apply_errata_setup_vm(void)
> +{
> + riscv_fill_cpu_mfr_info();
> +
> + switch (cpu_mfr_info.vendor_id) {
> +#ifdef CONFIG_ERRATA_THEAD
> + case THEAD_VENDOR_ID:
> + thead_errata_setup_vm(cpu_mfr_info.arch_id,
> + cpu_mfr_info.imp_id);
> + break;
> +#endif
> + default:
> + break;
> + }
> +}
> diff --git a/arch/riscv/errata/thead/Makefile b/arch/riscv/errata/thead/Makefile
> new file mode 100644
> index 000000000000..2d644e19caef
> --- /dev/null
> +++ b/arch/riscv/errata/thead/Makefile
> @@ -0,0 +1 @@
> +obj-y += errata.o
> diff --git a/arch/riscv/errata/thead/errata.c b/arch/riscv/errata/thead/errata.c
> new file mode 100644
> index 000000000000..1f5c0f82bc23
> --- /dev/null
> +++ b/arch/riscv/errata/thead/errata.c
> @@ -0,0 +1,47 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <linux/kernel.h>
> +#include <linux/string.h>
> +#include <linux/bug.h>
> +#include <asm/patch.h>
> +#include <asm/alternative.h>
> +#include <asm/vendorid_list.h>
> +#include <asm/errata_list.h>
> +#include <asm/pgtable-bits.h>
> +
> +#ifdef CONFIG_64BIT
> +/*
> + * T-HEAD C9xx PTE format:
> + * | 63 | 62 | 61 | 60 | 59-8 | 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0
> + * SO C B SH RSW D A G U X W R V
> + * ^ ^ ^ ^ ^
> + * BIT(63): SO - Strong Order
> + * BIT(62): C - Cacheable
> + * BIT(61): B - Bufferable
> + * BIT(60): SH - Shareable
> + *
> + * MT_MASK : [63 - 59]
> + * MT_PMA : C + SH
> + * MT_NC : (none)
> + * MT_IO : SO
> + */
> +#undef _PAGE_MT_MASK
> +#undef _PAGE_MT_PMA
> +#undef _PAGE_MT_NC
> +#undef _PAGE_MT_IO
> +
> +#define _PAGE_MT_MASK 0xf800000000000000
> +#define _PAGE_MT_PMA 0x5000000000000000
> +#define _PAGE_MT_NC 0x0
> +#define _PAGE_MT_IO 0x8000000000000000
> +#endif
> +
> +void __init thead_errata_setup_vm(unsigned long archid, unsigned long impid)
> +{
> +#ifdef CONFIG_64BIT
> + __riscv_pbmt.mask = _PAGE_MT_MASK;
> + __riscv_pbmt.mt[MT_PMA] = _PAGE_MT_PMA;
> + __riscv_pbmt.mt[MT_NC] = _PAGE_MT_NC;
> + __riscv_pbmt.mt[MT_IO] = _PAGE_MT_IO;
> +#endif
> +}
> diff --git a/arch/riscv/include/asm/alternative.h b/arch/riscv/include/asm/alternative.h
> index e625d3cafbed..3605894081a8 100644
> --- a/arch/riscv/include/asm/alternative.h
> +++ b/arch/riscv/include/asm/alternative.h
> @@ -18,6 +18,7 @@
> #include <asm/hwcap.h>
>
> void __init apply_boot_alternatives(void);
> +void __init apply_errata_setup_vm(void);
>
> struct alt_entry {
> void *old_ptr; /* address of original instruciton or data */
> @@ -35,5 +36,6 @@ struct errata_checkfunc_id {
> void sifive_errata_patch_func(struct alt_entry *begin, struct alt_entry *end,
> unsigned long archid, unsigned long impid);
>
> +void thead_errata_setup_vm(unsigned long archid, unsigned long impid);
> #endif
> #endif
> diff --git a/arch/riscv/include/asm/fixmap.h b/arch/riscv/include/asm/fixmap.h
> index 54cbf07fb4e9..5acd99d08e74 100644
> --- a/arch/riscv/include/asm/fixmap.h
> +++ b/arch/riscv/include/asm/fixmap.h
> @@ -43,7 +43,7 @@ enum fixed_addresses {
> __end_of_fixed_addresses
> };
>
> -#define FIXMAP_PAGE_IO PAGE_KERNEL
> +#define FIXMAP_PAGE_IO PAGE_IOREMAP
>
> #define __early_set_fixmap __set_fixmap
>
> diff --git a/arch/riscv/include/asm/pgtable-64.h b/arch/riscv/include/asm/pgtable-64.h
> index 228261aa9628..0b53ea67e91a 100644
> --- a/arch/riscv/include/asm/pgtable-64.h
> +++ b/arch/riscv/include/asm/pgtable-64.h
> @@ -61,12 +61,14 @@ static inline void pud_clear(pud_t *pudp)
>
> static inline pmd_t *pud_pgtable(pud_t pud)
> {
> - return (pmd_t *)pfn_to_virt(pud_val(pud) >> _PAGE_PFN_SHIFT);
> + return (pmd_t *)pfn_to_virt((pud_val(pud) & _PAGE_CHG_MASK)
> + >> _PAGE_PFN_SHIFT);
> }
>
> static inline struct page *pud_page(pud_t pud)
> {
> - return pfn_to_page(pud_val(pud) >> _PAGE_PFN_SHIFT);
> + return pfn_to_page((pud_val(pud) & _PAGE_CHG_MASK)
> + >> _PAGE_PFN_SHIFT);
> }
>
> static inline pmd_t pfn_pmd(unsigned long pfn, pgprot_t prot)
> @@ -76,7 +78,7 @@ static inline pmd_t pfn_pmd(unsigned long pfn, pgprot_t prot)
>
> static inline unsigned long _pmd_pfn(pmd_t pmd)
> {
> - return pmd_val(pmd) >> _PAGE_PFN_SHIFT;
> + return (pmd_val(pmd) & _PAGE_CHG_MASK) >> _PAGE_PFN_SHIFT;
> }
>
> #define mk_pmd(page, prot) pfn_pmd(page_to_pfn(page), prot)
> diff --git a/arch/riscv/include/asm/pgtable-bits.h b/arch/riscv/include/asm/pgtable-bits.h
> index 2ee413912926..38a33e076714 100644
> --- a/arch/riscv/include/asm/pgtable-bits.h
> +++ b/arch/riscv/include/asm/pgtable-bits.h
> @@ -7,7 +7,7 @@
> #define _ASM_RISCV_PGTABLE_BITS_H
>
> /*
> - * PTE format:
> + * rv32 PTE format:
> * | XLEN-1 10 | 9 8 | 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0
> * PFN reserved for SW D A G U X W R V
> */
> @@ -24,6 +24,47 @@
> #define _PAGE_DIRTY (1 << 7) /* Set by hardware on any write */
> #define _PAGE_SOFT (1 << 8) /* Reserved for software */
>
> +#ifndef __ASSEMBLY__
> +#ifdef CONFIG_64BIT
> +/*
> + * rv64 PTE format:
> + * | 63 | 62 61 | 60 54 | 53 10 | 9 8 | 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0
> + * N MT RSV PFN reserved for SW D A G U X W R V
> + * [62:61] Memory Type definitions:
> + * 00 - PMA Normal Cacheable, No change to implied PMA memory type
> + * 01 - NC Non-cacheable, idempotent, weakly-ordered Main Memory
> + * 10 - IO Non-cacheable, non-idempotent, strongly-ordered I/O memory
> + * 11 - Rsvd Reserved for future standard use
> + */
> +#define _PAGE_MT_MASK ((u64)0x3 << 61)
> +#define _PAGE_MT_PMA ((u64)0x0 << 61)
> +#define _PAGE_MT_NC ((u64)0x1 << 61)
> +#define _PAGE_MT_IO ((u64)0x2 << 61)
> +
> +enum {
> + MT_PMA,
> + MT_NC,
> + MT_IO,
> + MT_MAX
> +};
> +
> +extern struct __riscv_pbmt_struct {
> + unsigned long mask;
> + unsigned long mt[MT_MAX];
> +} __riscv_pbmt;
> +
> +#define _PAGE_DMA_MASK __riscv_pbmt.mask
> +#define _PAGE_DMA_PMA __riscv_pbmt.mt[MT_PMA]
> +#define _PAGE_DMA_NC __riscv_pbmt.mt[MT_NC]
> +#define _PAGE_DMA_IO __riscv_pbmt.mt[MT_IO]
> +#else
> +#define _PAGE_DMA_MASK 0
> +#define _PAGE_DMA_PMA 0
> +#define _PAGE_DMA_NC 0
> +#define _PAGE_DMA_IO 0
> +#endif /* CONFIG_64BIT */
> +#endif /* __ASSEMBLY__ */
> +
> #define _PAGE_SPECIAL _PAGE_SOFT
> #define _PAGE_TABLE _PAGE_PRESENT
>
> @@ -38,7 +79,8 @@
> /* Set of bits to preserve across pte_modify() */
> #define _PAGE_CHG_MASK (~(unsigned long)(_PAGE_PRESENT | _PAGE_READ | \
> _PAGE_WRITE | _PAGE_EXEC | \
> - _PAGE_USER | _PAGE_GLOBAL))
> + _PAGE_USER | _PAGE_GLOBAL | \
> + _PAGE_DMA_MASK))
> /*
> * when all of R/W/X are zero, the PTE is a pointer to the next level
> * of the page table; otherwise, it is a leaf PTE.
> diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
> index 39b550310ec6..dcde56f2046e 100644
> --- a/arch/riscv/include/asm/pgtable.h
> +++ b/arch/riscv/include/asm/pgtable.h
> @@ -115,7 +115,7 @@
> #define USER_PTRS_PER_PGD (TASK_SIZE / PGDIR_SIZE)
>
> /* Page protection bits */
> -#define _PAGE_BASE (_PAGE_PRESENT | _PAGE_ACCESSED | _PAGE_USER)
> +#define _PAGE_BASE (_PAGE_PRESENT | _PAGE_ACCESSED | _PAGE_USER | _PAGE_DMA_PMA)
>
> #define PAGE_NONE __pgprot(_PAGE_PROT_NONE)
> #define PAGE_READ __pgprot(_PAGE_BASE | _PAGE_READ)
> @@ -136,7 +136,8 @@
> | _PAGE_PRESENT \
> | _PAGE_ACCESSED \
> | _PAGE_DIRTY \
> - | _PAGE_GLOBAL)
> + | _PAGE_GLOBAL \
> + | _PAGE_DMA_PMA)
>
> #define PAGE_KERNEL __pgprot(_PAGE_KERNEL)
> #define PAGE_KERNEL_READ __pgprot(_PAGE_KERNEL & ~_PAGE_WRITE)
> @@ -146,11 +147,9 @@
>
> #define PAGE_TABLE __pgprot(_PAGE_TABLE)
>
> -/*
> - * The RISC-V ISA doesn't yet specify how to query or modify PMAs, so we can't
> - * change the properties of memory regions.
> - */
> -#define _PAGE_IOREMAP _PAGE_KERNEL
> +#define _PAGE_IOREMAP ((_PAGE_KERNEL & ~_PAGE_DMA_MASK) | _PAGE_DMA_IO)
> +
> +#define PAGE_IOREMAP __pgprot(_PAGE_IOREMAP)
>
> extern pgd_t swapper_pg_dir[];
>
> @@ -230,12 +229,12 @@ static inline unsigned long _pgd_pfn(pgd_t pgd)
>
> static inline struct page *pmd_page(pmd_t pmd)
> {
> - return pfn_to_page(pmd_val(pmd) >> _PAGE_PFN_SHIFT);
> + return pfn_to_page((pmd_val(pmd) & _PAGE_CHG_MASK) >> _PAGE_PFN_SHIFT);
> }
>
> static inline unsigned long pmd_page_vaddr(pmd_t pmd)
> {
> - return (unsigned long)pfn_to_virt(pmd_val(pmd) >> _PAGE_PFN_SHIFT);
> + return (unsigned long)pfn_to_virt((pmd_val(pmd) & _PAGE_CHG_MASK) >> _PAGE_PFN_SHIFT);
> }
>
> static inline pte_t pmd_pte(pmd_t pmd)
> @@ -251,7 +250,7 @@ static inline pte_t pud_pte(pud_t pud)
> /* Yields the page frame number (PFN) of a page table entry */
> static inline unsigned long pte_pfn(pte_t pte)
> {
> - return (pte_val(pte) >> _PAGE_PFN_SHIFT);
> + return ((pte_val(pte) & _PAGE_CHG_MASK) >> _PAGE_PFN_SHIFT);
> }
>
> #define pte_page(x) pfn_to_page(pte_pfn(x))
> @@ -490,6 +489,17 @@ static inline int ptep_clear_flush_young(struct vm_area_struct *vma,
> return ptep_test_and_clear_young(vma, address, ptep);
> }
>
> +#define pgprot_noncached pgprot_noncached
> +static inline pgprot_t pgprot_noncached(pgprot_t _prot)
> +{
> + unsigned long prot = pgprot_val(_prot);
> +
> + prot &= ~_PAGE_DMA_MASK;
> + prot |= _PAGE_DMA_IO;
> +
> + return __pgprot(prot);
> +}
> +
> /*
> * THP functions
> */
> diff --git a/arch/riscv/include/asm/vendorid_list.h b/arch/riscv/include/asm/vendorid_list.h
> index 9d934215b3c8..bdfce1a4b42b 100644
> --- a/arch/riscv/include/asm/vendorid_list.h
> +++ b/arch/riscv/include/asm/vendorid_list.h
> @@ -6,5 +6,6 @@
> #define ASM_VENDOR_LIST_H
>
> #define SIFIVE_VENDOR_ID 0x489
> +#define THEAD_VENDOR_ID 0x5b7
>
> #endif
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index 975c7322d6c4..ee6dba4e49b5 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -577,6 +577,7 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
> #ifndef __PAGETABLE_PMD_FOLDED
> pmd_t fix_bmap_spmd, fix_bmap_epmd;
> #endif
> + apply_errata_setup_vm();

We probably should call this only if pbmt extension is present. We
need to dynamically determine
the presence of PBMT anyways because DIRECT_REMAP will be enabled for
all platforms (single defconfig).
We need to think about all the following possible platforms with
different types of cache coherent I/O devices.

1. platform supporting standard PBMT extensions (future ones)
2. platform with custom PBMT extension (Allwinner D1)
3. platform with uncached window approach (starlight)
4. platform with fully cache coherent IO (hifive unleashed/unmatched)

I think a simple global flag based on a DT property should be
sufficient. DMA generic code may also need to check
if the platform supports direct_remap in addition to checking the
config option.

For the DT property, we should define a generic extensible DT binding
for ISA extensions so that future ISA extensions
should be able to leverage that.

Just food for thought for RISC-V MC at plumbers next week.

>
> setup_protection_map();
>
> @@ -927,3 +928,8 @@ int __meminit vmemmap_populate(unsigned long start, unsigned long end, int node,
> return vmemmap_populate_basepages(start, end, node, NULL);
> }
> #endif
> +
> +#ifdef CONFIG_64BIT
> +struct __riscv_pbmt_struct __riscv_pbmt __ro_after_init;
> +EXPORT_SYMBOL(__riscv_pbmt);
> +#endif
> --
> 2.25.1
>
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv



--
Regards,
Atish