From: Guo Ren <[email protected]>
The current riscv linux implementation requires SOC system to support
memory coherence between all I/O devices and CPUs. But some SOC systems
cannot maintain the coherence and they need support cache clean/invalid
operations to synchronize data.
Current implementation is no problem with SiFive FU540, because FU540
keeps all IO devices and DMA master devices coherence with CPU. But to a
traditional SOC vendor, it may already have a stable non-coherency SOC
system, the need is simply to replace the CPU with RV CPU and rebuild
the whole system with IO-coherency is very expensive.
So we should make riscv linux also support non-coherency memory model.
Here are the two points that riscv linux needs to be modified:
- Add _PAGE_COHERENCY bit in current page table entry attributes. The bit
designates a coherence for this page mapping. Software set the bit to
tell the hardware that the region of the page's memory area must be
coherent with IOs devices in SOC system by PMA settings.
If IOs and CPU are already coherent in SOC system, CPU just ignore
this bit.
PTE format:
| XLEN-1 10 | 9 | 8 | 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0
PFN C RSW D A G U X W R V
^
BIT(9): Coherence attribute bit
0: hardware needn't keep the page coherenct and software will
maintain the coherence with cache clear/invalid operations.
1: hardware must keep the page coherenct and software needn't
maintain the coherence.
BIT(8): Reserved for software and now it's _PAGE_SPECIAL in linux
Add a new hardware bit in PTE also need to modify Privileged
Architecture Supervisor-Level ISA:
https://github.com/riscv/riscv-isa-manual/pull/374
- Add SBI_FENCE_DMA 9 in riscv-sbi.
sbi_fence_dma(start, size, dir) could synchronize CPU cache data with
DMA device in non-coherency memory model. The third param's definition
is the same with linux's in include/linux/dma-direction.h:
enum dma_data_direction {
DMA_BIDIRECTIONAL = 0,
DMA_TO_DEVICE = 1,
DMA_FROM_DEVICE = 2,
DMA_NONE = 3,
};
The first param:start must be physical address which could be handled
in M-state.
Here is a pull request to the riscv-sbi-doc:
https://github.com/riscv/riscv-sbi-doc/pull/15
We have tested the patch on our fpga SOC system which network controller
connected to a non-cache-coherency interconnect in and it couldn't work
without the patch.
There is no side effect for FU540 whose CPU don't care _PAGE_COHERENCY
in PTE, but FU540's bbl also need to implement a simple sbi_fence_dma
by directly return. In fact, if you give a correct configuration for
dev_is_dma_conherent(), linux dma framework wouldn't call sbi_fence_dma
any more.
Changelog:
- Use coherency instead of consistency for all to maintain term
consistency. (Xiang Xiaoyan)
- Add riscv-isa-manual modification pull request link.
- Correct grammatical errors.
Signed-off-by: Guo Ren <[email protected]>
Cc: Andrew Waterman <[email protected]>
Cc: Anup Patel <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Greentime Hu <[email protected]>
Cc: Marek Szyprowski <[email protected]>
Cc: Mike Rapoport <[email protected]>
Cc: Palmer Dabbelt <[email protected]>
Cc: Robin Murphy <[email protected]>
Cc: Scott Wood <[email protected]>
Cc: Vincent Chen <[email protected]>
Cc: Xiang Xiaoyan <[email protected]>
---
arch/riscv/Kconfig | 4 ++++
arch/riscv/include/asm/pgtable-bits.h | 1 +
arch/riscv/include/asm/pgtable.h | 11 +++++++++
arch/riscv/include/asm/sbi.h | 10 ++++++++
arch/riscv/mm/Makefile | 1 +
arch/riscv/mm/dma-mapping.c | 44 +++++++++++++++++++++++++++++++++++
arch/riscv/mm/ioremap.c | 2 +-
7 files changed, 72 insertions(+), 1 deletion(-)
create mode 100644 arch/riscv/mm/dma-mapping.c
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index eb56c82..f0fc503 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -16,9 +16,12 @@ config RISCV
select OF
select OF_EARLY_FLATTREE
select OF_IRQ
+ select ARCH_HAS_SYNC_DMA_FOR_CPU
+ select ARCH_HAS_SYNC_DMA_FOR_DEVICE
select ARCH_WANT_FRAME_POINTERS
select CLONE_BACKWARDS
select COMMON_CLK
+ select DMA_DIRECT_REMAP
select GENERIC_CLOCKEVENTS
select GENERIC_CPU_DEVICES
select GENERIC_IRQ_SHOW
@@ -27,6 +30,7 @@ config RISCV
select GENERIC_STRNCPY_FROM_USER
select GENERIC_STRNLEN_USER
select GENERIC_SMP_IDLE_THREAD
+ select GENERIC_ALLOCATOR
select GENERIC_ATOMIC64 if !64BIT || !RISCV_ISA_A
select HAVE_ARCH_AUDITSYSCALL
select HAVE_MEMBLOCK_NODE_MAP
diff --git a/arch/riscv/include/asm/pgtable-bits.h b/arch/riscv/include/asm/pgtable-bits.h
index 470755c..104f8c0 100644
--- a/arch/riscv/include/asm/pgtable-bits.h
+++ b/arch/riscv/include/asm/pgtable-bits.h
@@ -31,6 +31,7 @@
#define _PAGE_ACCESSED (1 << 6) /* Set by hardware on any access */
#define _PAGE_DIRTY (1 << 7) /* Set by hardware on any write */
#define _PAGE_SOFT (1 << 8) /* Reserved for software */
+#define _PAGE_COHERENCY (1 << 9) /* Coherency */
#define _PAGE_SPECIAL _PAGE_SOFT
#define _PAGE_TABLE _PAGE_PRESENT
diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
index 1141364..26debb4 100644
--- a/arch/riscv/include/asm/pgtable.h
+++ b/arch/riscv/include/asm/pgtable.h
@@ -66,6 +66,7 @@
#define PAGE_KERNEL __pgprot(_PAGE_KERNEL)
#define PAGE_KERNEL_EXEC __pgprot(_PAGE_KERNEL | _PAGE_EXEC)
+#define PAGE_KERNEL_COHERENCY __pgprot(_PAGE_KERNEL | _PAGE_COHERENCY)
extern pgd_t swapper_pg_dir[];
@@ -375,6 +376,16 @@ 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_COHERENCY;
+
+ return __pgprot(prot);
+}
+
/*
* Encode and decode a swap entry
*
diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h
index b6bb10b..b945e50 100644
--- a/arch/riscv/include/asm/sbi.h
+++ b/arch/riscv/include/asm/sbi.h
@@ -25,6 +25,7 @@
#define SBI_REMOTE_SFENCE_VMA 6
#define SBI_REMOTE_SFENCE_VMA_ASID 7
#define SBI_SHUTDOWN 8
+#define SBI_FENCE_DMA 9
#define SBI_CALL(which, arg0, arg1, arg2) ({ \
register uintptr_t a0 asm ("a0") = (uintptr_t)(arg0); \
@@ -42,6 +43,8 @@
#define SBI_CALL_0(which) SBI_CALL(which, 0, 0, 0)
#define SBI_CALL_1(which, arg0) SBI_CALL(which, arg0, 0, 0)
#define SBI_CALL_2(which, arg0, arg1) SBI_CALL(which, arg0, arg1, 0)
+#define SBI_CALL_3(which, arg0, arg1, arg2) \
+ SBI_CALL(which, arg0, arg1, arg2)
static inline void sbi_console_putchar(int ch)
{
@@ -82,6 +85,13 @@ static inline void sbi_remote_fence_i(const unsigned long *hart_mask)
SBI_CALL_1(SBI_REMOTE_FENCE_I, hart_mask);
}
+static inline void sbi_fence_dma(unsigned long start,
+ unsigned long size,
+ unsigned long dir)
+{
+ SBI_CALL_3(SBI_FENCE_DMA, start, size, dir);
+}
+
static inline void sbi_remote_sfence_vma(const unsigned long *hart_mask,
unsigned long start,
unsigned long size)
diff --git a/arch/riscv/mm/Makefile b/arch/riscv/mm/Makefile
index b68aac7..adc563a 100644
--- a/arch/riscv/mm/Makefile
+++ b/arch/riscv/mm/Makefile
@@ -9,3 +9,4 @@ obj-y += fault.o
obj-y += extable.o
obj-y += ioremap.o
obj-y += cacheflush.o
+obj-y += dma-mapping.o
diff --git a/arch/riscv/mm/dma-mapping.c b/arch/riscv/mm/dma-mapping.c
new file mode 100644
index 0000000..5e1d179
--- /dev/null
+++ b/arch/riscv/mm/dma-mapping.c
@@ -0,0 +1,44 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/dma-mapping.h>
+
+static int __init atomic_pool_init(void)
+{
+ return dma_atomic_pool_init(GFP_KERNEL, pgprot_noncached(PAGE_KERNEL));
+}
+postcore_initcall(atomic_pool_init);
+
+void arch_dma_prep_coherent(struct page *page, size_t size)
+{
+ memset(page_address(page), 0, size);
+
+ sbi_fence_dma(page_to_phys(page), size, DMA_BIDIRECTIONAL);
+}
+
+void arch_sync_dma_for_device(struct device *dev, phys_addr_t paddr,
+ size_t size, enum dma_data_direction dir)
+{
+ switch (dir) {
+ case DMA_TO_DEVICE:
+ case DMA_FROM_DEVICE:
+ case DMA_BIDIRECTIONAL:
+ sbi_fence_dma(paddr, size, dir);
+ break;
+ default:
+ BUG();
+ }
+}
+
+void arch_sync_dma_for_cpu(struct device *dev, phys_addr_t paddr,
+ size_t size, enum dma_data_direction dir)
+{
+ switch (dir) {
+ case DMA_TO_DEVICE:
+ case DMA_FROM_DEVICE:
+ case DMA_BIDIRECTIONAL:
+ sbi_fence_dma(paddr, size, dir);
+ break;
+ default:
+ BUG();
+ }
+}
diff --git a/arch/riscv/mm/ioremap.c b/arch/riscv/mm/ioremap.c
index bd2f2db..f6aaf1e 100644
--- a/arch/riscv/mm/ioremap.c
+++ b/arch/riscv/mm/ioremap.c
@@ -73,7 +73,7 @@ static void __iomem *__ioremap_caller(phys_addr_t addr, size_t size,
*/
void __iomem *ioremap(phys_addr_t offset, unsigned long size)
{
- return __ioremap_caller(offset, size, PAGE_KERNEL,
+ return __ioremap_caller(offset, size, PAGE_KERNEL_COHERENCY,
__builtin_return_address(0));
}
EXPORT_SYMBOL(ioremap);
--
2.7.4
On Mon, Apr 22, 2019 at 11:44:30PM +0800, [email protected] wrote:
> - Add _PAGE_COHERENCY bit in current page table entry attributes. The bit
> designates a coherence for this page mapping. Software set the bit to
> tell the hardware that the region of the page's memory area must be
> coherent with IOs devices in SOC system by PMA settings.
> If IOs and CPU are already coherent in SOC system, CPU just ignore
> this bit.
>
> PTE format:
> | XLEN-1 10 | 9 | 8 | 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0
> PFN C RSW D A G U X W R V
> ^
> BIT(9): Coherence attribute bit
> 0: hardware needn't keep the page coherenct and software will
> maintain the coherence with cache clear/invalid operations.
> 1: hardware must keep the page coherenct and software needn't
> maintain the coherence.
> BIT(8): Reserved for software and now it's _PAGE_SPECIAL in linux
>
> Add a new hardware bit in PTE also need to modify Privileged
> Architecture Supervisor-Level ISA:
> https://github.com/riscv/riscv-isa-manual/pull/374
>
> - Add SBI_FENCE_DMA 9 in riscv-sbi.
> sbi_fence_dma(start, size, dir) could synchronize CPU cache data with
> DMA device in non-coherency memory model. The third param's definition
> is the same with linux's in include/linux/dma-direction.h:
Please don't make this an SBI call. We need a proper instruction
for cache flushing and invalidation. We'll also need that for pmem
support for example. I heard at least one other vendor already
had an instruction, and we really need to get this into the privileged
spec ASAP (yesterday in fact).
If you have your own instructions already we can probably binary
patch those in using the Linux alternatives mechanism once we have
a standardized way in the privileged spec.
We should probably start a working group for this ASAP unless we can
get another working group to help taking care of it.
> +#define pgprot_noncached pgprot_noncached
> +static inline pgprot_t pgprot_noncached(pgprot_t _prot)
> +{
> + unsigned long prot = pgprot_val(_prot);
> +
> + prot |= _PAGE_COHERENCY;
> +
> + return __pgprot(prot);
Nitpick: this can be shortened to
return __pgprot(pgprot_val(prot) | _PAGE_COHERENCY));
Also is this really a coherent flag, or an 'uncached' flag like in
many other architectures?
> +++ b/arch/riscv/mm/dma-mapping.c
This should probably be called dma-noncoherent.c
It should also have a user visible config option so that we don't
have to build it for fully coherent systems.
> +void arch_dma_prep_coherent(struct page *page, size_t size)
> +{
> + memset(page_address(page), 0, size);
No need for this memset, the caller takes care of it.
> diff --git a/arch/riscv/mm/ioremap.c b/arch/riscv/mm/ioremap.c
> index bd2f2db..f6aaf1e 100644
> --- a/arch/riscv/mm/ioremap.c
> +++ b/arch/riscv/mm/ioremap.c
> @@ -73,7 +73,7 @@ static void __iomem *__ioremap_caller(phys_addr_t addr, size_t size,
> */
> void __iomem *ioremap(phys_addr_t offset, unsigned long size)
> {
> - return __ioremap_caller(offset, size, PAGE_KERNEL,
> + return __ioremap_caller(offset, size, PAGE_KERNEL_COHERENCY,
> __builtin_return_address(0));
> }
> EXPORT_SYMBOL(ioremap);
I think ioremap is a different story, and should be a separate patch.
Thx Christoph,
On Mon, Apr 22, 2019 at 06:18:14PM +0200, Christoph Hellwig wrote:
> On Mon, Apr 22, 2019 at 11:44:30PM +0800, [email protected] wrote:
> > - Add _PAGE_COHERENCY bit in current page table entry attributes. The bit
> > designates a coherence for this page mapping. Software set the bit to
> > tell the hardware that the region of the page's memory area must be
> > coherent with IOs devices in SOC system by PMA settings.
> > If IOs and CPU are already coherent in SOC system, CPU just ignore
> > this bit.
> >
> > PTE format:
> > | XLEN-1 10 | 9 | 8 | 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0
> > PFN C RSW D A G U X W R V
> > ^
> > BIT(9): Coherence attribute bit
> > 0: hardware needn't keep the page coherenct and software will
> > maintain the coherence with cache clear/invalid operations.
> > 1: hardware must keep the page coherenct and software needn't
> > maintain the coherence.
> > BIT(8): Reserved for software and now it's _PAGE_SPECIAL in linux
> >
> > Add a new hardware bit in PTE also need to modify Privileged
> > Architecture Supervisor-Level ISA:
> > https://github.com/riscv/riscv-isa-manual/pull/374
> >
> > - Add SBI_FENCE_DMA 9 in riscv-sbi.
> > sbi_fence_dma(start, size, dir) could synchronize CPU cache data with
> > DMA device in non-coherency memory model. The third param's definition
> > is the same with linux's in include/linux/dma-direction.h:
>
> Please don't make this an SBI call. We need a proper instruction
> for cache flushing and invalidation. We'll also need that for pmem
> support for example. I heard at least one other vendor already
> had an instruction, and we really need to get this into the privileged
> spec ASAP (yesterday in fact).
>
> If you have your own instructions already we can probably binary
> patch those in using the Linux alternatives mechanism once we have
> a standardized way in the privileged spec.
>
> We should probably start a working group for this ASAP unless we can
> get another working group to help taking care of it.
Good news, I prefer to use instructions directly instead of SBI_CALL.
Our instruction is "dcache.c/iva %0" (one cache line) and the parameter is
virtual address in S-state. When get into M-state by SBI_CALL, we could
let dcache.c/iva use physical addres directly and it needn't kmap page
for RV32 with highmem (Of cause highmem is not ready in RV32 now).
>
> > +#define pgprot_noncached pgprot_noncached
> > +static inline pgprot_t pgprot_noncached(pgprot_t _prot)
> > +{
> > + unsigned long prot = pgprot_val(_prot);
> > +
> > + prot |= _PAGE_COHERENCY;
> > +
> > + return __pgprot(prot);
>
> Nitpick: this can be shortened to
>
> return __pgprot(pgprot_val(prot) | _PAGE_COHERENCY));
Good.
>
> Also is this really a coherent flag, or an 'uncached' flag like in
> many other architectures?
There are a lot of features about coherency attributes, eg: cacheable,
bufferable, strong order ..., and coherency is a more abstract name to
contain all of these. In our hardware, coherence = uncached +
unbufferable + (stong order).
But I'm not very care about the name is, uncached is also ok. My key
point is the bits of page attributes is very precious and this patch
will use the last unused attribute bit in PTE.
Another point is we could get more attribute bits by modify the riscv
spec:
- Remove Global bit, I think it's duplicate with the User bit in linux.
- Change _PAGE_PFN_SHIFT from 10 to 12, because the huge pfn in RV32 is
very useless and current RV32 linux doesn't even implement highmem.
And then we could get another three page attribute bits in PTE.
>
> > +++ b/arch/riscv/mm/dma-mapping.c
>
> This should probably be called dma-noncoherent.c
>
> It should also have a user visible config option so that we don't
> have to build it for fully coherent systems.
Ok, dma-noncoherent.c is more clear.
>
> > +void arch_dma_prep_coherent(struct page *page, size_t size)
> > +{
> > + memset(page_address(page), 0, size);
>
> No need for this memset, the caller takes care of it.
Ok
>
> > diff --git a/arch/riscv/mm/ioremap.c b/arch/riscv/mm/ioremap.c
> > index bd2f2db..f6aaf1e 100644
> > --- a/arch/riscv/mm/ioremap.c
> > +++ b/arch/riscv/mm/ioremap.c
> > @@ -73,7 +73,7 @@ static void __iomem *__ioremap_caller(phys_addr_t addr, size_t size,
> > */
> > void __iomem *ioremap(phys_addr_t offset, unsigned long size)
> > {
> > - return __ioremap_caller(offset, size, PAGE_KERNEL,
> > + return __ioremap_caller(offset, size, PAGE_KERNEL_COHERENCY,
> > __builtin_return_address(0));
> > }
> > EXPORT_SYMBOL(ioremap);
>
> I think ioremap is a different story, and should be a separate patch.
Ok
Best Regards
Guo Ren
Hi,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v5.1-rc6 next-20190418]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/guoren-kernel-org/riscv-Support-non-coherency-memory-model/20190423-075013
config: riscv-tinyconfig (attached as .config)
compiler: riscv64-linux-gcc (GCC) 8.1.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=8.1.0 make.cross ARCH=riscv
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <[email protected]>
All errors (new ones prefixed by >>):
arch/riscv/mm/dma-mapping.c: In function 'arch_dma_prep_coherent':
>> arch/riscv/mm/dma-mapping.c:15:2: error: implicit declaration of function 'sbi_fence_dma' [-Werror=implicit-function-declaration]
sbi_fence_dma(page_to_phys(page), size, DMA_BIDIRECTIONAL);
^~~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +/sbi_fence_dma +15 arch/riscv/mm/dma-mapping.c
10
11 void arch_dma_prep_coherent(struct page *page, size_t size)
12 {
13 memset(page_address(page), 0, size);
14
> 15 sbi_fence_dma(page_to_phys(page), size, DMA_BIDIRECTIONAL);
16 }
17
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Tue, Apr 23, 2019 at 08:13:48AM +0800, Guo Ren wrote:
> > We should probably start a working group for this ASAP unless we can
> > get another working group to help taking care of it.
> Good news, I prefer to use instructions directly instead of SBI_CALL.
>
> Our instruction is "dcache.c/iva %0" (one cache line) and the parameter is
> virtual address in S-state. When get into M-state by SBI_CALL, we could
> let dcache.c/iva use physical addres directly and it needn't kmap page
> for RV32 with highmem (Of cause highmem is not ready in RV32 now).
So you only have one instruction variant? Normally we'd have two or
three to implement the non-coherent DMA (or pmem) semantics:
cache writeback, cache invalidate and potentially cache writeback +
invalidate to optimize that case. Here is the table how Linux
uses them for DMA:
| map == for_device | unmap == for_cpu
|----------------------------------------------------------------
TO_DEV | writeback writeback | none none
FROM_DEV | invalidate invalidate | invalidate* invalidate*
BIDI | writeback+inv writeback+inv | invalidate invalidate
[*] needed for CPU speculative prefetches
We already have a discussion on isa-dev on something like these
instructions:
https://groups.google.com/a/groups.riscv.org/forum/#!msg/isa-dev/qXbzqaQbDXU/4ThcEAeCCAAJ
It got a little side tracked, both due to the usual noise on isa-dev
and due to the proposal including a lot more instructions that might be
a little more contentious, but it might be a good start to bring this
into a working group.
> > Also is this really a coherent flag, or an 'uncached' flag like in
> > many other architectures?
> There are a lot of features about coherency attributes, eg: cacheable,
> bufferable, strong order ..., and coherency is a more abstract name to
> contain all of these. In our hardware, coherence = uncached +
> unbufferable + (stong order).
>
> But I'm not very care about the name is, uncached is also ok. My key
> point is the bits of page attributes is very precious and this patch
> will use the last unused attribute bit in PTE.
I don't care about the name actually, more about having defined semantics.
Totally uncached should include unbuffered. I don't think we need the
strong ordering for DMA memory either.
> Another point is we could get more attribute bits by modify the riscv
> spec:
> - Remove Global bit, I think it's duplicate with the User bit in linux.
It is in Linux, but it is conceptually very different.
> - Change _PAGE_PFN_SHIFT from 10 to 12, because the huge pfn in RV32 is
> very useless and current RV32 linux doesn't even implement highmem.
This would seem sensible to me, but I'm not sure everyone agrees. Even
then we are very late in the game for changes like that.
On Tue, Apr 23, 2019 at 07:55:48AM +0200, Christoph Hellwig wrote:
> On Tue, Apr 23, 2019 at 08:13:48AM +0800, Guo Ren wrote:
> > > We should probably start a working group for this ASAP unless we can
> > > get another working group to help taking care of it.
> > Good news, I prefer to use instructions directly instead of SBI_CALL.
> >
> > Our instruction is "dcache.c/iva %0" (one cache line) and the parameter is
> > virtual address in S-state. When get into M-state by SBI_CALL, we could
> > let dcache.c/iva use physical addres directly and it needn't kmap page
> > for RV32 with highmem (Of cause highmem is not ready in RV32 now).
>
> So you only have one instruction variant? Normally we'd have two or
> three to implement the non-coherent DMA (or pmem) semantics:
dcache.c/iva means three instructions:
- dcache.cva %0 : writeback by virtual address cacheline
- dcache.iva %0 : invalid by virtual address cacheline
- dcache.civa %0 : writeback+inv by virtual address cacheline
We also have memory barrier instructions, ref:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/csky/include/asm/barrier.h
>
> cache writeback, cache invalidate and potentially cache writeback +
> invalidate to optimize that case. Here is the table how Linux
> uses them for DMA:
>
> | map == for_device | unmap == for_cpu
> |----------------------------------------------------------------
> TO_DEV | writeback writeback | none none
> FROM_DEV | invalidate invalidate | invalidate* invalidate*
> BIDI | writeback+inv writeback+inv | invalidate invalidate
>
> [*] needed for CPU speculative prefetches
>
>
> We already have a discussion on isa-dev on something like these
> instructions:
>
> https://groups.google.com/a/groups.riscv.org/forum/#!msg/isa-dev/qXbzqaQbDXU/4ThcEAeCCAAJ
>
> It got a little side tracked, both due to the usual noise on isa-dev
> and due to the proposal including a lot more instructions that might be
> a little more contentious, but it might be a good start to bring this
> into a working group.
I think using sbi_call as a temporary solution is a good choice before
the instruction is determined.
>
> > > Also is this really a coherent flag, or an 'uncached' flag like in
> > > many other architectures?
> > There are a lot of features about coherency attributes, eg: cacheable,
> > bufferable, strong order ..., and coherency is a more abstract name to
> > contain all of these. In our hardware, coherence = uncached +
> > unbufferable + (stong order).
> >
> > But I'm not very care about the name is, uncached is also ok. My key
> > point is the bits of page attributes is very precious and this patch
> > will use the last unused attribute bit in PTE.
>
> I don't care about the name actually, more about having defined semantics.
> Totally uncached should include unbuffered. I don't think we need the
> strong ordering for DMA memory either.
Yes, memory don't need strong order and strong order in PMA also implies
uncached, unbuffered for IO address. If the entry of PMA is strong
order, the page mapping must be uncached + unbuffered + strong-order
without _PAGE_COHENCY in PTE.
>
> > Another point is we could get more attribute bits by modify the riscv
> > spec:
> > - Remove Global bit, I think it's duplicate with the User bit in linux.
>
> It is in Linux, but it is conceptually very different.
Yes, but hardware could ignore one of them and in riscv linux
_PAGE_GLOBAL is no use at all, see:
grep _PAGE_GLOBAL arch/riscv -r
In fact, the _PAGE_KERNEL for pte doesn't contain _PAGE_GLOBAL and it
works on FU540 and qemu. As I've mentioned page attribute bits is very
precious, define a useless bit make people confused.
>
> > - Change _PAGE_PFN_SHIFT from 10 to 12, because the huge pfn in RV32 is
> > very useless and current RV32 linux doesn't even implement highmem.
>
> This would seem sensible to me, but I'm not sure everyone agrees. Even
> then we are very late in the game for changes like that.
Agree.
Best Regards
Guo Ren
On 23/04/2019 16:46, Guo Ren wrote:
> On Tue, Apr 23, 2019 at 07:55:48AM +0200, Christoph Hellwig wrote:
>> On Tue, Apr 23, 2019 at 08:13:48AM +0800, Guo Ren wrote:
>>>> We should probably start a working group for this ASAP unless we can
>>>> get another working group to help taking care of it.
>>> Good news, I prefer to use instructions directly instead of SBI_CALL.
>>>
>>> Our instruction is "dcache.c/iva %0" (one cache line) and the parameter is
>>> virtual address in S-state. When get into M-state by SBI_CALL, we could
>>> let dcache.c/iva use physical addres directly and it needn't kmap page
>>> for RV32 with highmem (Of cause highmem is not ready in RV32 now).
>>
>> So you only have one instruction variant? Normally we'd have two or
>> three to implement the non-coherent DMA (or pmem) semantics:
> dcache.c/iva means three instructions:
> - dcache.cva %0 : writeback by virtual address cacheline
> - dcache.iva %0 : invalid by virtual address cacheline
> - dcache.civa %0 : writeback+inv by virtual address cacheline
>
> We also have memory barrier instructions, ref:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/csky/include/asm/barrier.h
>
>>
>> cache writeback, cache invalidate and potentially cache writeback +
>> invalidate to optimize that case. Here is the table how Linux
>> uses them for DMA:
>>
>> | map == for_device | unmap == for_cpu
>> |----------------------------------------------------------------
>> TO_DEV | writeback writeback | none none
>> FROM_DEV | invalidate invalidate | invalidate* invalidate*
>> BIDI | writeback+inv writeback+inv | invalidate invalidate
>>
>> [*] needed for CPU speculative prefetches
>>
>>
>> We already have a discussion on isa-dev on something like these
>> instructions:
>>
>> https://groups.google.com/a/groups.riscv.org/forum/#!msg/isa-dev/qXbzqaQbDXU/4ThcEAeCCAAJ
>>
>> It got a little side tracked, both due to the usual noise on isa-dev
>> and due to the proposal including a lot more instructions that might be
>> a little more contentious, but it might be a good start to bring this
>> into a working group.
> I think using sbi_call as a temporary solution is a good choice before
> the instruction is determined.
>
>>
>>>> Also is this really a coherent flag, or an 'uncached' flag like in
>>>> many other architectures?
>>> There are a lot of features about coherency attributes, eg: cacheable,
>>> bufferable, strong order ..., and coherency is a more abstract name to
>>> contain all of these. In our hardware, coherence = uncached +
>>> unbufferable + (stong order).
>>>
>>> But I'm not very care about the name is, uncached is also ok. My key
>>> point is the bits of page attributes is very precious and this patch
>>> will use the last unused attribute bit in PTE.
>>
>> I don't care about the name actually, more about having defined semantics.
>> Totally uncached should include unbuffered. I don't think we need the
>> strong ordering for DMA memory either.
> Yes, memory don't need strong order and strong order in PMA also implies
> uncached, unbuffered for IO address. If the entry of PMA is strong
> order, the page mapping must be uncached + unbuffered + strong-order
> without _PAGE_COHENCY in PTE.
>
>>
>>> Another point is we could get more attribute bits by modify the riscv
>>> spec:
>>> - Remove Global bit, I think it's duplicate with the User bit in linux.
>>
>> It is in Linux, but it is conceptually very different.
> Yes, but hardware could ignore one of them and in riscv linux
> _PAGE_GLOBAL is no use at all, see:
> grep _PAGE_GLOBAL arch/riscv -r
>
> In fact, the _PAGE_KERNEL for pte doesn't contain _PAGE_GLOBAL and it
> works on FU540 and qemu. As I've mentioned page attribute bits is very
> precious, define a useless bit make people confused.
>
The fact that it isn't used yet doesn't imply it is not useful. We don't
use ASIDs at the moment, and without using ASIDs the "global" bit is
indeed not useful. However with ASIDs the bit will be vital for saving
TLB spaces. Without the global bit, the kernel pages become synonyms to
themselves (i.e. they have different tags in TLB but refer to the same
physical page).
The global bit also exists in many other ISAs as well. It's definitely
not a "useless" bits.
Moreover, this bit is already implemented in both Rocket and Ariane. It
is also in the spec for quite a while. The fact that Linux doesn't use
it at the moment is not a reason for removing it.
>
>>
>>> - Change _PAGE_PFN_SHIFT from 10 to 12, because the huge pfn in RV32 is
>>> very useless and current RV32 linux doesn't even implement highmem.
>>
>> This would seem sensible to me, but I'm not sure everyone agrees. Even
>> then we are very late in the game for changes like that.
> Agree.
>
> Best Regards
> Guo Ren
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv
>
Hi Gary,
On Tue, Apr 23, 2019 at 03:57:30PM +0000, Gary Guo wrote:
> >>> Another point is we could get more attribute bits by modify the riscv
> >>> spec:
> >>> - Remove Global bit, I think it's duplicate with the User bit in linux.
> >>
> >> It is in Linux, but it is conceptually very different.
> > Yes, but hardware could ignore one of them and in riscv linux
> > _PAGE_GLOBAL is no use at all, see:
> > grep _PAGE_GLOBAL arch/riscv -r
> >
> > In fact, the _PAGE_KERNEL for pte doesn't contain _PAGE_GLOBAL and it
> > works on FU540 and qemu. As I've mentioned page attribute bits is very
> > precious, define a useless bit make people confused.
> >
>
> The fact that it isn't used yet doesn't imply it is not useful. We don't
> use ASIDs at the moment, and without using ASIDs the "global" bit is
> indeed not useful. However with ASIDs the bit will be vital for saving
> TLB spaces. Without the global bit, the kernel pages become synonyms to
> themselves (i.e. they have different tags in TLB but refer to the same
> physical page).
>
> The global bit also exists in many other ISAs as well. It's definitely
> not a "useless" bits.
>
> Moreover, this bit is already implemented in both Rocket and Ariane. It
> is also in the spec for quite a while. The fact that Linux doesn't use
> it at the moment is not a reason for removing it.
>
Look:
linux-next git:(riscv_asid_allocator_v2)$ grep GLOBAL arch/riscv -r
arch/riscv/include/asm/pgtable-bits.h:#define _PAGE_GLOBAL (1 << 5) /* Global */
arch/riscv/include/asm/pgtable-bits.h: _PAGE_USER | _PAGE_GLOBAL))
Your patch tell us _PAGE_USER and _PAGE_GLOBAL are duplicate and why we
couldn't make _PAGE_USER implies _PAGE_GLOBAL? Can you give an example
of a real scene in PTE about:
_PAGE_USER:0 + _PAGE_GLOBAL:1
or
_PAGE_USER:1 + _PAGE_GLOBAL:0
Of cause I know USER & GLOBAL are conceptually very different, but
there are only 10 attribute-bits for riscv (In fact we've wasted two bits
to support huge RV32-pfn :P). So I think it is time to merge these two bits
before hardware supports GLOBAL. Reserve them for future!
Best Regards
Guo Ren
> -----Original Message-----
> From: Guo Ren <[email protected]>
> Sent: Wednesday, April 24, 2019 03:08
> To: Gary Guo <[email protected]>
> Cc: Christoph Hellwig <[email protected]>; [email protected]; Palmer
> Dabbelt <[email protected]>; Andrew Waterman <[email protected]>; Arnd
> Bergmann <[email protected]>; Anup Patel <[email protected]>; Xiang
> Xiaoyan <[email protected]>; [email protected]; Mike
> Rapoport <[email protected]>; Vincent Chen <[email protected]>;
> Greentime Hu <[email protected]>; [email protected]; linux-
> [email protected]; Marek Szyprowski <[email protected]>;
> Robin Murphy <[email protected]>; Scott Wood <[email protected]>;
> [email protected]
> Subject: Re: [PATCH] riscv: Support non-coherency memory model
>
> Hi Gary,
>
> On Tue, Apr 23, 2019 at 03:57:30PM +0000, Gary Guo wrote:
> > >>> Another point is we could get more attribute bits by modify the riscv
> > >>> spec:
> > >>> - Remove Global bit, I think it's duplicate with the User bit in linux.
> > >>
> > >> It is in Linux, but it is conceptually very different.
> > > Yes, but hardware could ignore one of them and in riscv linux
> > > _PAGE_GLOBAL is no use at all, see:
> > > grep _PAGE_GLOBAL arch/riscv -r
> > >
> > > In fact, the _PAGE_KERNEL for pte doesn't contain _PAGE_GLOBAL and it
> > > works on FU540 and qemu. As I've mentioned page attribute bits is very
> > > precious, define a useless bit make people confused.
> > >
> >
> > The fact that it isn't used yet doesn't imply it is not useful. We don't
> > use ASIDs at the moment, and without using ASIDs the "global" bit is
> > indeed not useful. However with ASIDs the bit will be vital for saving
> > TLB spaces. Without the global bit, the kernel pages become synonyms to
> > themselves (i.e. they have different tags in TLB but refer to the same
> > physical page).
> >
> > The global bit also exists in many other ISAs as well. It's definitely
> > not a "useless" bits.
> >
> > Moreover, this bit is already implemented in both Rocket and Ariane. It
> > is also in the spec for quite a while. The fact that Linux doesn't use
> > it at the moment is not a reason for removing it.
> >
>
> Look:
> linux-next git:(riscv_asid_allocator_v2)$ grep GLOBAL arch/riscv -r
> arch/riscv/include/asm/pgtable-bits.h:#define _PAGE_GLOBAL (1 << 5) /*
> Global */
> arch/riscv/include/asm/pgtable-bits.h: _PAGE_USER |
> _PAGE_GLOBAL))
>
> Your patch tell us _PAGE_USER and _PAGE_GLOBAL are duplicate and why we
> couldn't make _PAGE_USER implies _PAGE_GLOBAL? Can you give an example
> of a real scene in PTE about:
> _PAGE_USER:0 + _PAGE_GLOBAL:1
> or
> _PAGE_USER:1 + _PAGE_GLOBAL:0
>
> Of cause I know USER & GLOBAL are conceptually very different, but
> there are only 10 attribute-bits for riscv (In fact we've wasted two bits
> to support huge RV32-pfn :P). So I think it is time to merge these two bits
> before hardware supports GLOBAL. Reserve them for future!
Two cases I can think of:
* vdso like things. They're user pages that can really be shared across address spaces (i.e. global). Kernels like L4 implement most systems calls similar to VDSO, so USER + GLOBAL is useful.
* hypervisor without H-extension: This requires shadow page tables. Supervisor pages are mapped to supervisor shadow pages. However these shadow pages cannot be GLOBAL because they can't be shared between VMs. So !USER + !GLOBAL is useful.
Remember Linux isn't the only supervisor software that RISC-V cares!
>
> Best Regards
> Guo Ren
Hi Gary,
On Wed, Apr 24, 2019 at 03:21:14AM +0000, Gary Guo wrote:
> > Look:
> > linux-next git:(riscv_asid_allocator_v2)$ grep GLOBAL arch/riscv -r
> > arch/riscv/include/asm/pgtable-bits.h:#define _PAGE_GLOBAL (1 << 5) /*
> > Global */
> > arch/riscv/include/asm/pgtable-bits.h: _PAGE_USER |
> > _PAGE_GLOBAL))
> >
> > Your patch tell us _PAGE_USER and _PAGE_GLOBAL are duplicate and why we
> > couldn't make _PAGE_USER implies _PAGE_GLOBAL? Can you give an example
> > of a real scene in PTE about:
> > _PAGE_USER:0 + _PAGE_GLOBAL:1
> > or
> > _PAGE_USER:1 + _PAGE_GLOBAL:0
> >
> > Of cause I know USER & GLOBAL are conceptually very different, but
> > there are only 10 attribute-bits for riscv (In fact we've wasted two bits
> > to support huge RV32-pfn :P). So I think it is time to merge these two bits
> > before hardware supports GLOBAL. Reserve them for future!
>
> Two cases I can think of:
> * vdso like things. They're user pages that can really be shared across address spaces (i.e. global). Kernels like L4 implement most systems calls similar to VDSO, so USER + GLOBAL is useful.
Vdso is a user space mapping in linux, See: fs/binfmt_elf.c
static int load_elf_binary(struct linux_binprm *bprm) {
...
#ifdef ARCH_HAS_SETUP_ADDITIONAL_PAGES
retval = arch_setup_additional_pages(bprm, !!elf_interpreter);
if (retval < 0)
goto out;
#endif /* ARCH_HAS_SETUP_ADDITIONAL_PAGES */
All linux archs use arch_setup_additional_pages for vdso mapping and
every process has its own vdso mapping to the same pages.
I don't think vdso is a real scene for GLOBAL in PTE.
> * hypervisor without H-extension: This requires shadow page tables. Supervisor
> pages are mapped to supervisor shadow pages. However these shadow pages cannot
> be GLOBAL because they can't be shared between VMs. So !USER + !GLOBAL is useful.
Hypervisor use 2-stages TLB translation in hardware and shadow page
tables is for stage 2 translation. Shadow page tables care vmid not
asid.
If hardware don't support H-extension (MMU 2-stages translation), it's
hard to accept for virtualization performance.
I don't think hypervisor is a real scene for GLOBAL in PTE.
Are there other scene for GLOBAL in PTE?
Best Regards
Guo Ren
On 24/04/2019 06:57, Guo Ren wrote:
> Hi Gary,
>
> On Wed, Apr 24, 2019 at 03:21:14AM +0000, Gary Guo wrote:
>>> Look:
>>> linux-next git:(riscv_asid_allocator_v2)$ grep GLOBAL arch/riscv -r
>>> arch/riscv/include/asm/pgtable-bits.h:#define _PAGE_GLOBAL (1 << 5) /*
>>> Global */
>>> arch/riscv/include/asm/pgtable-bits.h: _PAGE_USER |
>>> _PAGE_GLOBAL))
>>>
>>> Your patch tell us _PAGE_USER and _PAGE_GLOBAL are duplicate and why we
>>> couldn't make _PAGE_USER implies _PAGE_GLOBAL? Can you give an example
>>> of a real scene in PTE about:
>>> _PAGE_USER:0 + _PAGE_GLOBAL:1
>>> or
>>> _PAGE_USER:1 + _PAGE_GLOBAL:0
>>>
>>> Of cause I know USER & GLOBAL are conceptually very different, but
>>> there are only 10 attribute-bits for riscv (In fact we've wasted two bits
>>> to support huge RV32-pfn :P). So I think it is time to merge these two bits
>>> before hardware supports GLOBAL. Reserve them for future!
>>
>> Two cases I can think of:
>> * vdso like things. They're user pages that can really be shared across address spaces (i.e. global). Kernels like L4 implement most systems calls similar to VDSO, so USER + GLOBAL is useful.
> Vdso is a user space mapping in linux, See: fs/binfmt_elf.c
>
> static int load_elf_binary(struct linux_binprm *bprm) {
> ...
> #ifdef ARCH_HAS_SETUP_ADDITIONAL_PAGES
> retval = arch_setup_additional_pages(bprm, !!elf_interpreter);
> if (retval < 0)
> goto out;
> #endif /* ARCH_HAS_SETUP_ADDITIONAL_PAGES */
>
> All linux archs use arch_setup_additional_pages for vdso mapping and
> every process has its own vdso mapping to the same pages.
But we shouldn't prevent a kernel from mapping a USER page globally. As
I said, the fact that Linux doesn't do it isn't a valid reason for
omitting the possibility.
>
> I don't think vdso is a real scene for GLOBAL in PTE.
>
>> * hypervisor without H-extension: This requires shadow page tables. Supervisor
>> pages are mapped to supervisor shadow pages. However these shadow pages cannot
>> be GLOBAL because they can't be shared between VMs. So !USER + !GLOBAL is useful.
> Hypervisor use 2-stages TLB translation in hardware and shadow page
> tables is for stage 2 translation. Shadow page tables care vmid not
> asid.
When H-extension is present, stage 2 translation uses VMID and is
performed by hardware. When H-extension is not present, there's no such
thing called VMID. When H-extension is not present, both hypervisor and
guest supervisor will run in supervisor mode, and hypervisor uses
MSTATUS.TVM to trap guest supervisor virtual memory operations. The
shadow page table is populated by doing 2-stage page walk in software.
In this case, the hypervisor likely needs to use some bits of ASID to
emulate the VMID feature. In this case GLOBAL page cannot be used as it
means that the page exists in all physical ASIDs (which contains both
emulated VMID and ASID). Having supervisor pages being GLOBAL makes the
semantics incorrect!
> If hardware don't support H-extension (MMU 2-stages translation), it's
> hard to accept for virtualization performance.
The RISC-V privileged spec is explicitly designed to allow the
techniques described above (this is the sole purpose of MSTATUS.TVM). It
might be as high performance as a hardware with H-extension, but is
definitely a legit use case. In fact, it is vital for use cases like
recursive virtualization.
Also, I believe the PTE format of RISC-V is already frozen -- therefore
it is impossible now to merge GLOBAL and USER bit, nor to replace RSW
bit with another bit.
>
> I don't think hypervisor is a real scene for GLOBAL in PTE.
>
> Are there other scene for GLOBAL in PTE?
>
> Best Regards
> Guo Ren
>
On Wed, Apr 24, 2019 at 12:45:56PM +0000, Gary Guo wrote:
> The RISC-V privileged spec is explicitly designed to allow the
> techniques described above (this is the sole purpose of MSTATUS.TVM). It
> might be as high performance as a hardware with H-extension, but is
> definitely a legit use case. In fact, it is vital for use cases like
> recursive virtualization.
>
> Also, I believe the PTE format of RISC-V is already frozen -- therefore
> it is impossible now to merge GLOBAL and USER bit, nor to replace RSW
> bit with another bit.
Yes, I do not think we can just repurpose a bit. Even using a currently
unused one would require some gymnastics.
That being said IFF we want to support non-coherent DMA (and I think we
do as people glue together their SOCs using shoestring and paper clips,
as already demonstrated by Andes and C-SKY in RISC-V space, and most
arm, mips and ppc SOCs) we need something like this flag. The current
RISC-V method that only allows M-mode to set up such attributes on
a small number or PMP regions just doesn't work well with the way how
Linux and most non-trivial OSes implement DMA memory allocations.
Note that I said well - in theory we can have a firmware provided
uncached pool - that is what Linux does on most nommu (that is without
pagetables) ports, but the fixed sized pool really does suck and will
make users very unhappy.
On Wed, Apr 24, 2019 at 4:23 PM Christoph Hellwig <[email protected]> wrote:
>
> On Wed, Apr 24, 2019 at 12:45:56PM +0000, Gary Guo wrote:
> > The RISC-V privileged spec is explicitly designed to allow the
> > techniques described above (this is the sole purpose of MSTATUS.TVM). It
> > might be as high performance as a hardware with H-extension, but is
> > definitely a legit use case. In fact, it is vital for use cases like
> > recursive virtualization.
> >
> > Also, I believe the PTE format of RISC-V is already frozen -- therefore
> > it is impossible now to merge GLOBAL and USER bit, nor to replace RSW
> > bit with another bit.
>
> Yes, I do not think we can just repurpose a bit. Even using a currently
> unused one would require some gymnastics.
>
> That being said IFF we want to support non-coherent DMA (and I think we
> do as people glue together their SOCs using shoestring and paper clips,
> as already demonstrated by Andes and C-SKY in RISC-V space, and most
> arm, mips and ppc SOCs) we need something like this flag. The current
> RISC-V method that only allows M-mode to set up such attributes on
> a small number or PMP regions just doesn't work well with the way how
> Linux and most non-trivial OSes implement DMA memory allocations.
>
> Note that I said well - in theory we can have a firmware provided
> uncached pool - that is what Linux does on most nommu (that is without
> pagetables) ports, but the fixed sized pool really does suck and will
> make users very unhappy.
You could probably get away with allowing uncached mappings only
for huge pages, and using one or two of the bits the PMD for it.
This should cover most use cases, since in practice coherent allocations
tend to be either small and rare (device descriptors) or very big
(frame buffer etc), and both cases can be handled with hugepages
and gen_pool_alloc, possibly CMA added in since there will likely
not be an IOMMU either on the systems that lack cache coherent DMA.
One downside is that you need a little more care for drivers that
use dma_mmap_coherent() to expose coherent buffers to user space.
Two other points about the proposal:
- Aside from completely uncached/unbuffered mappings, you typically
want uncached/buffered mappings to cover dma_alloc_wc() that is
typically used for frame buffers etc that need write-combining to get
acceptable performance
- you need to decide what is supposed to happen when there are
multiple conflicting mappings for the same physical address.
Arnd
Hi Arnd,
On Thu, Apr 25, 2019 at 11:50:11AM +0200, Arnd Bergmann wrote:
> On Wed, Apr 24, 2019 at 4:23 PM Christoph Hellwig <[email protected]> wrote:
> >
> > On Wed, Apr 24, 2019 at 12:45:56PM +0000, Gary Guo wrote:
> > > The RISC-V privileged spec is explicitly designed to allow the
> > > techniques described above (this is the sole purpose of MSTATUS.TVM). It
> > > might be as high performance as a hardware with H-extension, but is
> > > definitely a legit use case. In fact, it is vital for use cases like
> > > recursive virtualization.
> > >
> > > Also, I believe the PTE format of RISC-V is already frozen -- therefore
> > > it is impossible now to merge GLOBAL and USER bit, nor to replace RSW
> > > bit with another bit.
> >
> > Yes, I do not think we can just repurpose a bit. Even using a currently
> > unused one would require some gymnastics.
> >
> > That being said IFF we want to support non-coherent DMA (and I think we
> > do as people glue together their SOCs using shoestring and paper clips,
> > as already demonstrated by Andes and C-SKY in RISC-V space, and most
> > arm, mips and ppc SOCs) we need something like this flag. The current
> > RISC-V method that only allows M-mode to set up such attributes on
> > a small number or PMP regions just doesn't work well with the way how
> > Linux and most non-trivial OSes implement DMA memory allocations.
> >
> > Note that I said well - in theory we can have a firmware provided
> > uncached pool - that is what Linux does on most nommu (that is without
> > pagetables) ports, but the fixed sized pool really does suck and will
> > make users very unhappy.
>
> You could probably get away with allowing uncached mappings only
> for huge pages, and using one or two of the bits the PMD for it.
> This should cover most use cases, since in practice coherent allocations
> tend to be either small and rare (device descriptors) or very big
> (frame buffer etc), and both cases can be handled with hugepages
> and gen_pool_alloc, possibly CMA added in since there will likely
> not be an IOMMU either on the systems that lack cache coherent DMA.
Generally attributs in huge-tlb-entry and leaf-tlb-entry should be the
same. Only put _PAGE_CACHE and _PAGE_BUF bits in huge-tlb-entry sounds
a bit strange.
The gen_pool_alloc only 256KB by default, but a huge tlb entry is 4MB.
Hardware couldn't setup vitual-4MB to a phys-256KB range mapping in TLB.
>
> One downside is that you need a little more care for drivers that
> use dma_mmap_coherent() to expose coherent buffers to user space.
>
> Two other points about the proposal:
> - Aside from completely uncached/unbuffered mappings, you typically
> want uncached/buffered mappings to cover dma_alloc_wc() that is
> typically used for frame buffers etc that need write-combining to get
> acceptable performance
I agree dma_alloc_wc is necessary, and we need add another more attribute
bit in PTE: _PAGE_BUF.
Perhaps using _PAGE_BUF + _PAGE_CACHE are better then _PAGE_CONHENCY.
> - you need to decide what is supposed to happen when there are
> multiple conflicting mappings for the same physical address.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
What's the mulitple confilcing mappings ?
Best Regards
Guo Ren
On Fri, Apr 26, 2019 at 6:06 PM Guo Ren <[email protected]> wrote:
> On Thu, Apr 25, 2019 at 11:50:11AM +0200, Arnd Bergmann wrote:
> > On Wed, Apr 24, 2019 at 4:23 PM Christoph Hellwig <[email protected]> wrote:
> >
> > You could probably get away with allowing uncached mappings only
> > for huge pages, and using one or two of the bits the PMD for it.
> > This should cover most use cases, since in practice coherent allocations
> > tend to be either small and rare (device descriptors) or very big
> > (frame buffer etc), and both cases can be handled with hugepages
> > and gen_pool_alloc, possibly CMA added in since there will likely
> > not be an IOMMU either on the systems that lack cache coherent DMA.
>
> Generally attributs in huge-tlb-entry and leaf-tlb-entry should be the
> same. Only put _PAGE_CACHE and _PAGE_BUF bits in huge-tlb-entry sounds
> a bit strange.
Well, the point is that we can't really change the meaning of the existing
low bits, but because of the alignment contraints on hugepages, the extra bits
are currently unused for hugepage TLBs.
There are other architectures that reuse the bits in clever ways, e.g.
allowing larger physical address ranges to be used with hugepages than
normal pages.
> The gen_pool_alloc only 256KB by default, but a huge tlb entry is 4MB.
> Hardware couldn't setup vitual-4MB to a phys-256KB range mapping in TLB.
I expect the size would be easily changed, as long as there is sufficient
physical memory. If the entire system has 32MB or less, setting 2MB aside
would have a fairly significant impact of course.
> > - you need to decide what is supposed to happen when there are
> > multiple conflicting mappings for the same physical address.
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> What's the mulitple confilcing mappings ?
I mean when you have the linear mapping as cacheable and another
mapping for the same physical page as uncacheable, and then access
virtual address in both. This is usually a bad idea, but architectures
go to different lengths to prevent it.
The safest way would be for the CPU to produce a checkstop as soon
as there are TLB entries for the same physical address but different
caching settings. You can also do that if you have a cache-bypassing
load/store that hits a live cache line.
The other extreme would be to not do anything special and try to come
up with sane behavior, e.g. allow accesses in both ways but ensure that
a cache-bypassing load/store always flushes and invalidates cache
lines for the same physical address before its access.
Arnd
On 4/26/19 11:42 AM, Arnd Bergmann wrote:
> EXTERNAL MAIL
>
>
> On Fri, Apr 26, 2019 at 6:06 PM Guo Ren <[email protected]> wrote:
>> On Thu, Apr 25, 2019 at 11:50:11AM +0200, Arnd Bergmann wrote:
>>> On Wed, Apr 24, 2019 at 4:23 PM Christoph Hellwig <[email protected]> wrote:
>>>
>>> You could probably get away with allowing uncached mappings only
>>> for huge pages, and using one or two of the bits the PMD for it.
>>> This should cover most use cases, since in practice coherent allocations
>>> tend to be either small and rare (device descriptors) or very big
>>> (frame buffer etc), and both cases can be handled with hugepages
>>> and gen_pool_alloc, possibly CMA added in since there will likely
>>> not be an IOMMU either on the systems that lack cache coherent DMA.
>>
>> Generally attributs in huge-tlb-entry and leaf-tlb-entry should be the
>> same. Only put _PAGE_CACHE and _PAGE_BUF bits in huge-tlb-entry sounds
>> a bit strange.
>
> Well, the point is that we can't really change the meaning of the existing
> low bits, but because of the alignment contraints on hugepages, the extra bits
> are currently unused for hugepage TLBs.
> There are other architectures that reuse the bits in clever ways, e.g.
> allowing larger physical address ranges to be used with hugepages than
> normal pages.
>
>> The gen_pool_alloc only 256KB by default, but a huge tlb entry is 4MB.
>> Hardware couldn't setup vitual-4MB to a phys-256KB range mapping in TLB.
>
> I expect the size would be easily changed, as long as there is sufficient
> physical memory. If the entire system has 32MB or less, setting 2MB aside
> would have a fairly significant impact of course.
>
>
>>> - you need to decide what is supposed to happen when there are
>>> multiple conflicting mappings for the same physical address.
>> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>> What's the mulitple confilcing mappings ?
>
> I mean when you have the linear mapping as cacheable and another
> mapping for the same physical page as uncacheable, and then access
> virtual address in both. This is usually a bad idea, but architectures
> go to different lengths to prevent it.
>
> The safest way would be for the CPU to produce a checkstop as soon
> as there are TLB entries for the same physical address but different
> caching settings. You can also do that if you have a cache-bypassing
> load/store that hits a live cache line.
The second one is probably do-able in most systems. But the first one
usually isn't. The TLB usually can't be looked up by physical address.
Bill
>
> The other extreme would be to not do anything special and try to come
> up with sane behavior, e.g. allow accesses in both ways but ensure that
> a cache-bypassing load/store always flushes and invalidates cache
> lines for the same physical address before its access.
>
> Arnd
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
>
> View/Reply Online (#395): https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.riscv.org_g_tech-2Dprivileged_message_395&d=DwIBaQ&c=aUq983L2pue2FqKFoP6PGHMJQyoJ7kl3s3GZ-_haXqY&r=AYJ4kbebphYpRw2lYDUDCk5w5Qa3-DR3bQnFjLVmM80&m=zf51zm7BmTNIb87ycEVTpGbwXV6ovRb4Rqy-BVXZ2F4&s=IWaqMk0fwM69syrjjmqzm5u3GeI_wWUYTJfBXCbxnWA&e=
> Mute This Topic: https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.riscv.org_mt_31344322_1677293&d=DwIBaQ&c=aUq983L2pue2FqKFoP6PGHMJQyoJ7kl3s3GZ-_haXqY&r=AYJ4kbebphYpRw2lYDUDCk5w5Qa3-DR3bQnFjLVmM80&m=zf51zm7BmTNIb87ycEVTpGbwXV6ovRb4Rqy-BVXZ2F4&s=BwJEwbhCkTjHFPfiAQs7CBgG1U6kqM7yZbpSWXPTOoU&e=
> Group Owner: [email protected]
> Unsubscribe: https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.riscv.org_g_tech-2Dprivileged_unsub&d=DwIBaQ&c=aUq983L2pue2FqKFoP6PGHMJQyoJ7kl3s3GZ-_haXqY&r=AYJ4kbebphYpRw2lYDUDCk5w5Qa3-DR3bQnFjLVmM80&m=zf51zm7BmTNIb87ycEVTpGbwXV6ovRb4Rqy-BVXZ2F4&s=vSG91zravGfrVN9_elxveQPGPYaNew0MyETUvHfKSEk&e= [[email protected]]
> -=-=-=-=-=-=-=-=-=-=-=-
>
On Mon, 22 Apr 2019 08:44:30 PDT (-0700), [email protected] wrote:
> From: Guo Ren <[email protected]>
>
> The current riscv linux implementation requires SOC system to support
> memory coherence between all I/O devices and CPUs. But some SOC systems
> cannot maintain the coherence and they need support cache clean/invalid
> operations to synchronize data.
>
> Current implementation is no problem with SiFive FU540, because FU540
> keeps all IO devices and DMA master devices coherence with CPU. But to a
> traditional SOC vendor, it may already have a stable non-coherency SOC
> system, the need is simply to replace the CPU with RV CPU and rebuild
> the whole system with IO-coherency is very expensive.
>
> So we should make riscv linux also support non-coherency memory model.
> Here are the two points that riscv linux needs to be modified:
>
> - Add _PAGE_COHERENCY bit in current page table entry attributes. The bit
> designates a coherence for this page mapping. Software set the bit to
> tell the hardware that the region of the page's memory area must be
> coherent with IOs devices in SOC system by PMA settings.
> If IOs and CPU are already coherent in SOC system, CPU just ignore
> this bit.
>
> PTE format:
> | XLEN-1 10 | 9 | 8 | 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0
> PFN C RSW D A G U X W R V
> ^
> BIT(9): Coherence attribute bit
> 0: hardware needn't keep the page coherenct and software will
> maintain the coherence with cache clear/invalid operations.
> 1: hardware must keep the page coherenct and software needn't
> maintain the coherence.
> BIT(8): Reserved for software and now it's _PAGE_SPECIAL in linux
>
> Add a new hardware bit in PTE also need to modify Privileged
> Architecture Supervisor-Level ISA:
> https://github.com/riscv/riscv-isa-manual/pull/374
This is a RISC-V ISA modification, which isn't really appropriate to suggest on
the kernel mailing lists. The right place to talk about this is at the RISC-V
foundation, which owns the ISA -- we can't change the hardware with a patch to
Linux :).
> - Add SBI_FENCE_DMA 9 in riscv-sbi.
> sbi_fence_dma(start, size, dir) could synchronize CPU cache data with
> DMA device in non-coherency memory model. The third param's definition
> is the same with linux's in include/linux/dma-direction.h:
>
> enum dma_data_direction {
> DMA_BIDIRECTIONAL = 0,
> DMA_TO_DEVICE = 1,
> DMA_FROM_DEVICE = 2,
> DMA_NONE = 3,
> };
>
> The first param:start must be physical address which could be handled
> in M-state.
>
> Here is a pull request to the riscv-sbi-doc:
> https://github.com/riscv/riscv-sbi-doc/pull/15
>
> We have tested the patch on our fpga SOC system which network controller
> connected to a non-cache-coherency interconnect in and it couldn't work
> without the patch.
>
> There is no side effect for FU540 whose CPU don't care _PAGE_COHERENCY
> in PTE, but FU540's bbl also need to implement a simple sbi_fence_dma
> by directly return. In fact, if you give a correct configuration for
> dev_is_dma_conherent(), linux dma framework wouldn't call sbi_fence_dma
> any more.
Non-coherent fences also need to be discussed as part of a RISC-V ISA
extension. I know people have expressed interest, but I don't know of a
working group that's already been set up.
>
> Changelog:
> - Use coherency instead of consistency for all to maintain term
> consistency. (Xiang Xiaoyan)
> - Add riscv-isa-manual modification pull request link.
> - Correct grammatical errors.
>
> Signed-off-by: Guo Ren <[email protected]>
> Cc: Andrew Waterman <[email protected]>
> Cc: Anup Patel <[email protected]>
> Cc: Arnd Bergmann <[email protected]>
> Cc: Christoph Hellwig <[email protected]>
> Cc: Greentime Hu <[email protected]>
> Cc: Marek Szyprowski <[email protected]>
> Cc: Mike Rapoport <[email protected]>
> Cc: Palmer Dabbelt <[email protected]>
> Cc: Robin Murphy <[email protected]>
> Cc: Scott Wood <[email protected]>
> Cc: Vincent Chen <[email protected]>
> Cc: Xiang Xiaoyan <[email protected]>
> ---
> arch/riscv/Kconfig | 4 ++++
> arch/riscv/include/asm/pgtable-bits.h | 1 +
> arch/riscv/include/asm/pgtable.h | 11 +++++++++
> arch/riscv/include/asm/sbi.h | 10 ++++++++
> arch/riscv/mm/Makefile | 1 +
> arch/riscv/mm/dma-mapping.c | 44 +++++++++++++++++++++++++++++++++++
> arch/riscv/mm/ioremap.c | 2 +-
> 7 files changed, 72 insertions(+), 1 deletion(-)
> create mode 100644 arch/riscv/mm/dma-mapping.c
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index eb56c82..f0fc503 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -16,9 +16,12 @@ config RISCV
> select OF
> select OF_EARLY_FLATTREE
> select OF_IRQ
> + select ARCH_HAS_SYNC_DMA_FOR_CPU
> + select ARCH_HAS_SYNC_DMA_FOR_DEVICE
> select ARCH_WANT_FRAME_POINTERS
> select CLONE_BACKWARDS
> select COMMON_CLK
> + select DMA_DIRECT_REMAP
> select GENERIC_CLOCKEVENTS
> select GENERIC_CPU_DEVICES
> select GENERIC_IRQ_SHOW
> @@ -27,6 +30,7 @@ config RISCV
> select GENERIC_STRNCPY_FROM_USER
> select GENERIC_STRNLEN_USER
> select GENERIC_SMP_IDLE_THREAD
> + select GENERIC_ALLOCATOR
> select GENERIC_ATOMIC64 if !64BIT || !RISCV_ISA_A
> select HAVE_ARCH_AUDITSYSCALL
> select HAVE_MEMBLOCK_NODE_MAP
> diff --git a/arch/riscv/include/asm/pgtable-bits.h b/arch/riscv/include/asm/pgtable-bits.h
> index 470755c..104f8c0 100644
> --- a/arch/riscv/include/asm/pgtable-bits.h
> +++ b/arch/riscv/include/asm/pgtable-bits.h
> @@ -31,6 +31,7 @@
> #define _PAGE_ACCESSED (1 << 6) /* Set by hardware on any access */
> #define _PAGE_DIRTY (1 << 7) /* Set by hardware on any write */
> #define _PAGE_SOFT (1 << 8) /* Reserved for software */
> +#define _PAGE_COHERENCY (1 << 9) /* Coherency */
>
> #define _PAGE_SPECIAL _PAGE_SOFT
> #define _PAGE_TABLE _PAGE_PRESENT
> diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
> index 1141364..26debb4 100644
> --- a/arch/riscv/include/asm/pgtable.h
> +++ b/arch/riscv/include/asm/pgtable.h
> @@ -66,6 +66,7 @@
>
> #define PAGE_KERNEL __pgprot(_PAGE_KERNEL)
> #define PAGE_KERNEL_EXEC __pgprot(_PAGE_KERNEL | _PAGE_EXEC)
> +#define PAGE_KERNEL_COHERENCY __pgprot(_PAGE_KERNEL | _PAGE_COHERENCY)
>
> extern pgd_t swapper_pg_dir[];
>
> @@ -375,6 +376,16 @@ 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_COHERENCY;
> +
> + return __pgprot(prot);
> +}
> +
> /*
> * Encode and decode a swap entry
> *
> diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h
> index b6bb10b..b945e50 100644
> --- a/arch/riscv/include/asm/sbi.h
> +++ b/arch/riscv/include/asm/sbi.h
> @@ -25,6 +25,7 @@
> #define SBI_REMOTE_SFENCE_VMA 6
> #define SBI_REMOTE_SFENCE_VMA_ASID 7
> #define SBI_SHUTDOWN 8
> +#define SBI_FENCE_DMA 9
>
> #define SBI_CALL(which, arg0, arg1, arg2) ({ \
> register uintptr_t a0 asm ("a0") = (uintptr_t)(arg0); \
> @@ -42,6 +43,8 @@
> #define SBI_CALL_0(which) SBI_CALL(which, 0, 0, 0)
> #define SBI_CALL_1(which, arg0) SBI_CALL(which, arg0, 0, 0)
> #define SBI_CALL_2(which, arg0, arg1) SBI_CALL(which, arg0, arg1, 0)
> +#define SBI_CALL_3(which, arg0, arg1, arg2) \
> + SBI_CALL(which, arg0, arg1, arg2)
>
> static inline void sbi_console_putchar(int ch)
> {
> @@ -82,6 +85,13 @@ static inline void sbi_remote_fence_i(const unsigned long *hart_mask)
> SBI_CALL_1(SBI_REMOTE_FENCE_I, hart_mask);
> }
>
> +static inline void sbi_fence_dma(unsigned long start,
> + unsigned long size,
> + unsigned long dir)
> +{
> + SBI_CALL_3(SBI_FENCE_DMA, start, size, dir);
> +}
> +
> static inline void sbi_remote_sfence_vma(const unsigned long *hart_mask,
> unsigned long start,
> unsigned long size)
> diff --git a/arch/riscv/mm/Makefile b/arch/riscv/mm/Makefile
> index b68aac7..adc563a 100644
> --- a/arch/riscv/mm/Makefile
> +++ b/arch/riscv/mm/Makefile
> @@ -9,3 +9,4 @@ obj-y += fault.o
> obj-y += extable.o
> obj-y += ioremap.o
> obj-y += cacheflush.o
> +obj-y += dma-mapping.o
> diff --git a/arch/riscv/mm/dma-mapping.c b/arch/riscv/mm/dma-mapping.c
> new file mode 100644
> index 0000000..5e1d179
> --- /dev/null
> +++ b/arch/riscv/mm/dma-mapping.c
> @@ -0,0 +1,44 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/dma-mapping.h>
> +
> +static int __init atomic_pool_init(void)
> +{
> + return dma_atomic_pool_init(GFP_KERNEL, pgprot_noncached(PAGE_KERNEL));
> +}
> +postcore_initcall(atomic_pool_init);
> +
> +void arch_dma_prep_coherent(struct page *page, size_t size)
> +{
> + memset(page_address(page), 0, size);
> +
> + sbi_fence_dma(page_to_phys(page), size, DMA_BIDIRECTIONAL);
> +}
> +
> +void arch_sync_dma_for_device(struct device *dev, phys_addr_t paddr,
> + size_t size, enum dma_data_direction dir)
> +{
> + switch (dir) {
> + case DMA_TO_DEVICE:
> + case DMA_FROM_DEVICE:
> + case DMA_BIDIRECTIONAL:
> + sbi_fence_dma(paddr, size, dir);
> + break;
> + default:
> + BUG();
> + }
> +}
> +
> +void arch_sync_dma_for_cpu(struct device *dev, phys_addr_t paddr,
> + size_t size, enum dma_data_direction dir)
> +{
> + switch (dir) {
> + case DMA_TO_DEVICE:
> + case DMA_FROM_DEVICE:
> + case DMA_BIDIRECTIONAL:
> + sbi_fence_dma(paddr, size, dir);
> + break;
> + default:
> + BUG();
> + }
> +}
> diff --git a/arch/riscv/mm/ioremap.c b/arch/riscv/mm/ioremap.c
> index bd2f2db..f6aaf1e 100644
> --- a/arch/riscv/mm/ioremap.c
> +++ b/arch/riscv/mm/ioremap.c
> @@ -73,7 +73,7 @@ static void __iomem *__ioremap_caller(phys_addr_t addr, size_t size,
> */
> void __iomem *ioremap(phys_addr_t offset, unsigned long size)
> {
> - return __ioremap_caller(offset, size, PAGE_KERNEL,
> + return __ioremap_caller(offset, size, PAGE_KERNEL_COHERENCY,
> __builtin_return_address(0));
> }
> EXPORT_SYMBOL(ioremap);
On Mon, Apr 29, 2019 at 01:11:43PM -0700, Palmer Dabbelt wrote:
> On Mon, 22 Apr 2019 08:44:30 PDT (-0700), [email protected] wrote:
> >From: Guo Ren <[email protected]>
> >
> >The current riscv linux implementation requires SOC system to support
> >memory coherence between all I/O devices and CPUs. But some SOC systems
> >cannot maintain the coherence and they need support cache clean/invalid
> >operations to synchronize data.
> >
> >Current implementation is no problem with SiFive FU540, because FU540
> >keeps all IO devices and DMA master devices coherence with CPU. But to a
> >traditional SOC vendor, it may already have a stable non-coherency SOC
> >system, the need is simply to replace the CPU with RV CPU and rebuild
> >the whole system with IO-coherency is very expensive.
> >
> >So we should make riscv linux also support non-coherency memory model.
> >Here are the two points that riscv linux needs to be modified:
> >
> > - Add _PAGE_COHERENCY bit in current page table entry attributes. The bit
> > designates a coherence for this page mapping. Software set the bit to
> > tell the hardware that the region of the page's memory area must be
> > coherent with IOs devices in SOC system by PMA settings.
> > If IOs and CPU are already coherent in SOC system, CPU just ignore
> > this bit.
> >
> > PTE format:
> > | XLEN-1 10 | 9 | 8 | 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0
> > PFN C RSW D A G U X W R V
> > ^
> > BIT(9): Coherence attribute bit
> > 0: hardware needn't keep the page coherenct and software will
> > maintain the coherence with cache clear/invalid operations.
> > 1: hardware must keep the page coherenct and software needn't
> > maintain the coherence.
> > BIT(8): Reserved for software and now it's _PAGE_SPECIAL in linux
> >
> > Add a new hardware bit in PTE also need to modify Privileged
> > Architecture Supervisor-Level ISA:
> > https://github.com/riscv/riscv-isa-manual/pull/374
>
> This is a RISC-V ISA modification, which isn't really appropriate to suggest on
> the kernel mailing lists. The right place to talk about this is at the RISC-V
> foundation, which owns the ISA -- we can't change the hardware with a patch to
> Linux :).
I just want a discussion and a wide discussion is good for all of us :)
>
> > - Add SBI_FENCE_DMA 9 in riscv-sbi.
> > sbi_fence_dma(start, size, dir) could synchronize CPU cache data with
> > DMA device in non-coherency memory model. The third param's definition
> > is the same with linux's in include/linux/dma-direction.h:
> >
> > enum dma_data_direction {
> > DMA_BIDIRECTIONAL = 0,
> > DMA_TO_DEVICE = 1,
> > DMA_FROM_DEVICE = 2,
> > DMA_NONE = 3,
> > };
> >
> > The first param:start must be physical address which could be handled
> > in M-state.
> >
> > Here is a pull request to the riscv-sbi-doc:
> > https://github.com/riscv/riscv-sbi-doc/pull/15
> >
> >We have tested the patch on our fpga SOC system which network controller
> >connected to a non-cache-coherency interconnect in and it couldn't work
> >without the patch.
> >
> >There is no side effect for FU540 whose CPU don't care _PAGE_COHERENCY
> >in PTE, but FU540's bbl also need to implement a simple sbi_fence_dma
> >by directly return. In fact, if you give a correct configuration for
> >dev_is_dma_conherent(), linux dma framework wouldn't call sbi_fence_dma
> >any more.
>
> Non-coherent fences also need to be discussed as part of a RISC-V ISA
^^^^^^^^^^^^ ^^^^^^
fences instructions? not page attributes?
> extension.
> I know people have expressed interest, but I don't know of a
> working group that's already been set up.
Is that mean current RISC-V ISA forces the SOC to be coherent memory model?
Best Regards
Guo Ren