* [PATCH] powerpc/powernv: Fix sparse data type warnings in pci-ioda.c
From: Paul Mackerras @ 2018-03-28 0:44 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Alexey Kardashevskiy
From: Gavin Shan <gwshan@linux.vnet.ibm.com>
The value passed to __raw_rm_writeq() and __raw_writeq() should be "u64"
and "unsigned long". This fixes warning reported by sparse:
gwshan@gwshan:~/sandbox/l$ make C=2 CF=-D__CHECK_ENDIAN__ \
arch/powerpc/platforms/powernv/pci-ioda.o
arch/powerpc/platforms/powernv/pci-ioda.c:1794:41: \
warning: incorrect type in argument 1 (different base types)
arch/powerpc/platforms/powernv/pci-ioda.c:1794:41: \
expected unsigned long long [unsigned] [usertype] val
arch/powerpc/platforms/powernv/pci-ioda.c:1794:41: \
got restricted __be64 [usertype] <noident>
arch/powerpc/platforms/powernv/pci-ioda.c:1796:38: \
warning: incorrect type in argument 1 (different base types)
arch/powerpc/platforms/powernv/pci-ioda.c:1796:38: \
expected unsigned long [unsigned] v
arch/powerpc/platforms/powernv/pci-ioda.c:1796:38: \
got restricted __be64 [usertype] <noident>
This also fixes another warning reported by sparse:
gwshan@gwshan:~/sandbox/l$ make C=2 CF=-D__CHECK_ENDIAN__ \
arch/powerpc/platforms/powernv/pci-ioda.o
:
arch/powerpc/platforms/powernv/pci-ioda.c:2647:45: \
warning: cast to restricted __be64
Signed-off-by: Gavin Shan <gwshan@linux.vnet.ibm.com>
Signed-off-by: Paul Mackerras <paulus@ozlabs.org>
---
diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c
index a6c92c7..71de087 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda.c
@@ -1976,9 +1976,11 @@ static void pnv_pci_p7ioc_tce_invalidate(struct iommu_table *tbl,
mb(); /* Ensure above stores are visible */
while (start <= end) {
if (rm)
- __raw_rm_writeq(cpu_to_be64(start), invalidate);
+ __raw_rm_writeq((__force u64)cpu_to_be64(start),
+ invalidate);
else
- __raw_writeq(cpu_to_be64(start), invalidate);
+ __raw_writeq((__force unsigned long)cpu_to_be64(start),
+ invalidate);
start += inc;
}
@@ -2055,9 +2057,10 @@ static void pnv_pci_phb3_tce_invalidate_entire(struct pnv_phb *phb, bool rm)
mb(); /* Ensure previous TCE table stores are visible */
if (rm)
- __raw_rm_writeq(cpu_to_be64(val), invalidate);
+ __raw_rm_writeq((__force u64)cpu_to_be64(val), invalidate);
else
- __raw_writeq(cpu_to_be64(val), invalidate);
+ __raw_writeq((__force unsigned long)cpu_to_be64(val),
+ invalidate);
}
static inline void pnv_pci_phb3_tce_invalidate_pe(struct pnv_ioda_pe *pe)
@@ -2067,7 +2070,7 @@ static inline void pnv_pci_phb3_tce_invalidate_pe(struct pnv_ioda_pe *pe)
unsigned long val = PHB3_TCE_KILL_INVAL_PE | (pe->pe_number & 0xFF);
mb(); /* Ensure above stores are visible */
- __raw_writeq(cpu_to_be64(val), invalidate);
+ __raw_writeq((__force unsigned long)cpu_to_be64(val), invalidate);
}
static void pnv_pci_phb3_tce_invalidate(struct pnv_ioda_pe *pe, bool rm,
@@ -2090,9 +2093,11 @@ static void pnv_pci_phb3_tce_invalidate(struct pnv_ioda_pe *pe, bool rm,
while (start <= end) {
if (rm)
- __raw_rm_writeq(cpu_to_be64(start), invalidate);
+ __raw_rm_writeq((__force u64)cpu_to_be64(start),
+ invalidate);
else
- __raw_writeq(cpu_to_be64(start), invalidate);
+ __raw_writeq((__force unsigned long)cpu_to_be64(start),
+ invalidate);
start += inc;
}
}
@@ -2864,7 +2869,8 @@ static void pnv_pci_ioda2_table_do_free_pages(__be64 *addr,
u64 *tmp = (u64 *) addr_ul;
for (i = 0; i < size; ++i) {
- unsigned long hpa = be64_to_cpu(tmp[i]);
+ unsigned long hpa =
+ be64_to_cpu((__force __be64)(tmp[i]));
if (!(hpa & (TCE_PCI_READ | TCE_PCI_WRITE)))
continue;
^ permalink raw reply related
* Re: RFC on writel and writel_relaxed
From: Linus Torvalds @ 2018-03-28 0:39 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Alexander Duyck, Will Deacon, Sinan Kaya, Arnd Bergmann,
Jason Gunthorpe, David Laight, Oliver,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT),
linux-rdma@vger.kernel.org, Alexander Duyck, Paul E. McKenney,
netdev@vger.kernel.org
In-Reply-To: <1522186396.7364.61.camel@kernel.crashing.org>
On Tue, Mar 27, 2018 at 11:33 AM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
>
> Well, we need to clarify that once and for all, because as I wrote
> earlier, it was decreed by Linus more than a decade ago that writel
> would be fully ordered by itself vs. previous memory stores (at least
> on UC memory).
Yes.
So "writel()" needs to be ordered with respect to other writel() uses
on the same thread. Anything else *will* break drivers. Obviously, the
drivers may then do magic to say "do write combining etc", but that
magic will be architecture-specific.
The other issue is that "writel()" needs to be ordered wrt other CPU's
doing "writel()" if those writel's are in a spinlocked region.
So it's not that "writel()" needs to be ordered wrt the spinlock
itself, but you *do* need to honor ordering if you have something like
this:
spin_lock(&somelock);
writel(a);
writel(b);
spin_unlock(&somelock);
and if two CPU's run the above code "at the same time", then the
*ONLY* acceptable sequence is abab.
You cannot, and must not, ever see "aabb" at the device, for example,
because of how the writel would basically leak out of the spinlock.
That sounds "obvious", but dammit, a lot of architectures got that
wrong, afaik.
Linus
^ permalink raw reply
* [PATCH v2 3/5] powerpc/mm/32: Use page_is_ram to check for RAM
From: Jonathan Neuschäfer @ 2018-03-28 0:25 UTC (permalink / raw)
To: linuxppc-dev
Cc: linux-kernel, Michael Ellerman, Christophe Leroy,
Jonathan Neuschäfer, Benjamin Herrenschmidt, Paul Mackerras,
Balbir Singh, Guenter Roeck
In-Reply-To: <20180328002544.18526-1-j.neuschaefer@gmx.net>
On systems where there is MMIO space between different blocks of RAM in
the physical address space, __ioremap_caller did not allow mapping these
MMIO areas, because they were below the end RAM and thus considered RAM
as well. Use the memblock-based page_is_ram function, which returns
false for such MMIO holes.
v2:
Keep the check for p < virt_to_phys(high_memory). On 32-bit systems
with high memory (memory above physical address 4GiB), the high memory
is expected to be available though ioremap. The high_memory variable
marks the end of low memory; comparing against it means that only
ioremap requests for low RAM will be denied.
Reported by Michael Ellerman.
Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
---
arch/powerpc/mm/pgtable_32.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/mm/pgtable_32.c b/arch/powerpc/mm/pgtable_32.c
index d35d9ad3c1cd..6668ecc041ad 100644
--- a/arch/powerpc/mm/pgtable_32.c
+++ b/arch/powerpc/mm/pgtable_32.c
@@ -148,6 +148,7 @@ __ioremap_caller(phys_addr_t addr, unsigned long size, unsigned long flags,
* mem_init() sets high_memory so only do the check after that.
*/
if (slab_is_available() && (p < virt_to_phys(high_memory)) &&
+ page_is_ram(__phys_to_pfn(p)) &&
!(__allow_ioremap_reserved && memblock_is_region_reserved(p, size))) {
printk("__ioremap(): phys addr 0x%llx is RAM lr %ps\n",
(unsigned long long)p, __builtin_return_address(0));
--
2.16.2
^ permalink raw reply related
* [PATCH v2 5/5] powerpc/mm/32: Remove the reserved memory hack
From: Jonathan Neuschäfer @ 2018-03-28 0:25 UTC (permalink / raw)
To: linuxppc-dev
Cc: linux-kernel, Michael Ellerman, Christophe Leroy,
Jonathan Neuschäfer, Benjamin Herrenschmidt, Paul Mackerras,
Aneesh Kumar K.V, Mathieu Malaterre, Balbir Singh, Guenter Roeck
In-Reply-To: <20180328002544.18526-1-j.neuschaefer@gmx.net>
This hack, introduced in commit c5df7f775148 ("powerpc: allow ioremap
within reserved memory regions") is now unnecessary.
Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
---
v2: no changes
---
arch/powerpc/mm/init_32.c | 5 -----
arch/powerpc/mm/mmu_decl.h | 1 -
arch/powerpc/mm/pgtable_32.c | 3 +--
3 files changed, 1 insertion(+), 8 deletions(-)
diff --git a/arch/powerpc/mm/init_32.c b/arch/powerpc/mm/init_32.c
index a2bf6965d04f..3e59e5d64b01 100644
--- a/arch/powerpc/mm/init_32.c
+++ b/arch/powerpc/mm/init_32.c
@@ -88,11 +88,6 @@ void MMU_init(void);
int __map_without_bats;
int __map_without_ltlbs;
-/*
- * This tells the system to allow ioremapping memory marked as reserved.
- */
-int __allow_ioremap_reserved;
-
/* max amount of low RAM to map in */
unsigned long __max_low_memory = MAX_LOW_MEM;
diff --git a/arch/powerpc/mm/mmu_decl.h b/arch/powerpc/mm/mmu_decl.h
index 57fbc554c785..c4c0a09a7775 100644
--- a/arch/powerpc/mm/mmu_decl.h
+++ b/arch/powerpc/mm/mmu_decl.h
@@ -98,7 +98,6 @@ extern void setbat(int index, unsigned long virt, phys_addr_t phys,
unsigned int size, pgprot_t prot);
extern int __map_without_bats;
-extern int __allow_ioremap_reserved;
extern unsigned int rtas_data, rtas_size;
struct hash_pte;
diff --git a/arch/powerpc/mm/pgtable_32.c b/arch/powerpc/mm/pgtable_32.c
index 6668ecc041ad..120a49bfb9c6 100644
--- a/arch/powerpc/mm/pgtable_32.c
+++ b/arch/powerpc/mm/pgtable_32.c
@@ -148,8 +148,7 @@ __ioremap_caller(phys_addr_t addr, unsigned long size, unsigned long flags,
* mem_init() sets high_memory so only do the check after that.
*/
if (slab_is_available() && (p < virt_to_phys(high_memory)) &&
- page_is_ram(__phys_to_pfn(p)) &&
- !(__allow_ioremap_reserved && memblock_is_region_reserved(p, size))) {
+ page_is_ram(__phys_to_pfn(p))) {
printk("__ioremap(): phys addr 0x%llx is RAM lr %ps\n",
(unsigned long long)p, __builtin_return_address(0));
return NULL;
--
2.16.2
^ permalink raw reply related
* [PATCH v2 2/5] powerpc: mm: Use memblock API for PPC32 page_is_ram
From: Jonathan Neuschäfer @ 2018-03-28 0:25 UTC (permalink / raw)
To: linuxppc-dev
Cc: linux-kernel, Michael Ellerman, Christophe Leroy,
Jonathan Neuschäfer, Benjamin Herrenschmidt, Paul Mackerras,
Michal Hocko, Andrew Morton, Vlastimil Babka, Dan Williams,
Joe Perches, Oliver O'Halloran
In-Reply-To: <20180328002544.18526-1-j.neuschaefer@gmx.net>
To support accurate checking for different blocks of memory on PPC32,
use the same memblock-based approach that's already used on PPC64 also
on PPC32.
Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
---
v2: no changes
---
arch/powerpc/mm/mem.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index da4e1555d61d..a42b86e2a34c 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -82,11 +82,7 @@ static inline pte_t *virt_to_kpte(unsigned long vaddr)
int page_is_ram(unsigned long pfn)
{
-#ifndef CONFIG_PPC64 /* XXX for now */
- return pfn < max_pfn;
-#else
return memblock_is_memory(__pfn_to_phys(pfn));
-#endif
}
pgprot_t phys_mem_access_prot(struct file *file, unsigned long pfn,
--
2.16.2
^ permalink raw reply related
* [PATCH v2 4/5] powerpc: wii: Don't rely on the reserved memory hack
From: Jonathan Neuschäfer @ 2018-03-28 0:25 UTC (permalink / raw)
To: linuxppc-dev
Cc: linux-kernel, Michael Ellerman, Christophe Leroy,
Jonathan Neuschäfer, Benjamin Herrenschmidt, Paul Mackerras
In-Reply-To: <20180328002544.18526-1-j.neuschaefer@gmx.net>
Because the two memory blocks (usually called MEM1 and MEM2) are not
merged anymore, __request_region in kernel/resource.c will correctly
allow reserving regions in the physical address space between MEM1 and
MEM2, where many important peripherals are (GPIO, MMC, USB, ...).
A previous change to __ioremap_caller in arch/powerpc/mm/pgtable_32.c
ensures that multiple memblocks are properly considered in ioremap; this
makes it unnecessary to set __allow_ioremap_reserved.
Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
---
v2:
Add some text to the commit message.
---
arch/powerpc/platforms/embedded6xx/wii.c | 14 +-------------
1 file changed, 1 insertion(+), 13 deletions(-)
diff --git a/arch/powerpc/platforms/embedded6xx/wii.c b/arch/powerpc/platforms/embedded6xx/wii.c
index 4682327f76a9..fc00d82691e1 100644
--- a/arch/powerpc/platforms/embedded6xx/wii.c
+++ b/arch/powerpc/platforms/embedded6xx/wii.c
@@ -81,21 +81,9 @@ void __init wii_memory_fixups(void)
BUG_ON(memblock.memory.cnt != 2);
BUG_ON(!page_aligned(p[0].base) || !page_aligned(p[1].base));
- /* trim unaligned tail */
- memblock_remove(ALIGN(p[1].base + p[1].size, PAGE_SIZE),
- (phys_addr_t)ULLONG_MAX);
-
- /* determine hole, add & reserve them */
+ /* determine hole */
wii_hole_start = ALIGN(p[0].base + p[0].size, PAGE_SIZE);
wii_hole_size = p[1].base - wii_hole_start;
- memblock_add(wii_hole_start, wii_hole_size);
- memblock_reserve(wii_hole_start, wii_hole_size);
-
- BUG_ON(memblock.memory.cnt != 1);
- __memblock_dump_all();
-
- /* allow ioremapping the address space in the hole */
- __allow_ioremap_reserved = 1;
}
unsigned long __init wii_mmu_mapin_mem2(unsigned long top)
--
2.16.2
^ permalink raw reply related
* [PATCH v2 1/5] powerpc: mm: Simplify page_is_ram by using memblock_is_memory
From: Jonathan Neuschäfer @ 2018-03-28 0:25 UTC (permalink / raw)
To: linuxppc-dev
Cc: linux-kernel, Michael Ellerman, Christophe Leroy,
Jonathan Neuschäfer, Benjamin Herrenschmidt, Paul Mackerras,
Michal Hocko, Andrew Morton, Vlastimil Babka, Joe Perches,
Oliver O'Halloran
In-Reply-To: <20180328002544.18526-1-j.neuschaefer@gmx.net>
Instead of open-coding the search in page_is_ram, call memblock_is_memory.
Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
---
v2: no changes
---
arch/powerpc/mm/mem.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index fe8c61149fb8..da4e1555d61d 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -85,13 +85,7 @@ int page_is_ram(unsigned long pfn)
#ifndef CONFIG_PPC64 /* XXX for now */
return pfn < max_pfn;
#else
- unsigned long paddr = (pfn << PAGE_SHIFT);
- struct memblock_region *reg;
-
- for_each_memblock(memory, reg)
- if (paddr >= reg->base && paddr < (reg->base + reg->size))
- return 1;
- return 0;
+ return memblock_is_memory(__pfn_to_phys(pfn));
#endif
}
--
2.16.2
^ permalink raw reply related
* [PATCH v2 0/5] PPC32/ioremap: Use memblock API to check for RAM
From: Jonathan Neuschäfer @ 2018-03-28 0:25 UTC (permalink / raw)
To: linuxppc-dev
Cc: linux-kernel, Michael Ellerman, Christophe Leroy,
Jonathan Neuschäfer
v1: https://www.spinics.net/lists/linux-mm/msg145939.html
This patchset makes it possible to allocate MMIO ranges that are between
the two RAM chunks on the Wii, MEM1 and MEM2, not only with ioremap
(which previously worked through a hack) but also with kernel/resource.c.
Changes in v2:
- I added back the p < virt_to_phys(high_memory) check in
__ioremap_caller because high memory should be allocatable through
ioremap
- I expanded the commit messages a bit
Jonathan Neuschäfer (5):
powerpc: mm: Simplify page_is_ram by using memblock_is_memory
powerpc: mm: Use memblock API for PPC32 page_is_ram
powerpc/mm/32: Use page_is_ram to check for RAM
powerpc: wii: Don't rely on the reserved memory hack
powerpc/mm/32: Remove the reserved memory hack
arch/powerpc/mm/init_32.c | 5 -----
arch/powerpc/mm/mem.c | 12 +-----------
arch/powerpc/mm/mmu_decl.h | 1 -
arch/powerpc/mm/pgtable_32.c | 2 +-
arch/powerpc/platforms/embedded6xx/wii.c | 14 +-------------
5 files changed, 3 insertions(+), 31 deletions(-)
--
2.16.2
^ permalink raw reply
* Re: [RFC PATCH v2 0/2] Randomization of address chosen by mmap.
From: Rich Felker @ 2018-03-28 0:00 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Kees Cook, Ilya Smith, Michal Hocko, Richard Henderson, ink,
mattst88, Vineet Gupta, Russell King, Tony Luck, Fenghua Yu,
Ralf Baechle, James E.J. Bottomley, Helge Deller,
Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Martin Schwidefsky, Heiko Carstens, Yoshinori Sato,
David S. Miller, Thomas Gleixner, Ingo Molnar, H. Peter Anvin,
X86 ML, nyc, Al Viro, Arnd Bergmann, Greg KH, Deepa Dinamani,
Hugh Dickins, Kate Stewart, Philippe Ombredanne, Andrew Morton,
Steve Capper, Punit Agrawal, Aneesh Kumar K.V, Nick Piggin,
Bhupesh Sharma, Rik van Riel, nitin.m.gupta, Kirill A. Shutemov,
Dan Williams, Jan Kara, Ross Zwisler, Jerome Glisse,
Andrea Arcangeli, Oleg Nesterov, linux-alpha, LKML,
linux-snps-arc, linux-ia64, linux-metag, Linux MIPS Mailing List,
linux-parisc, PowerPC, linux-s390, linux-sh, sparclinux, Linux-MM
In-Reply-To: <20180327234904.GA27734@bombadil.infradead.org>
On Tue, Mar 27, 2018 at 04:49:04PM -0700, Matthew Wilcox wrote:
> On Tue, Mar 27, 2018 at 03:53:53PM -0700, Kees Cook wrote:
> > I agree: pushing this off to libc leaves a lot of things unprotected.
> > I think this should live in the kernel. The question I have is about
> > making it maintainable/readable/etc.
> >
> > The state-of-the-art for ASLR is moving to finer granularity (over
> > just base-address offset), so I'd really like to see this supported in
> > the kernel. We'll be getting there for other things in the future, and
> > I'd like to have a working production example for researchers to
> > study, etc.
>
> One thing we need is to limit the fragmentation of this approach.
> Even on 64-bit systems, we can easily get into a situation where there isn't
> space to map a contiguous terabyte.
The default limit of only 65536 VMAs will also quickly come into play
if consecutive anon mmaps don't get merged. Of course this can be
raised, but it has significant resource and performance (fork) costs.
Rich
^ permalink raw reply
* Re: [RFC PATCH v2 0/2] Randomization of address chosen by mmap.
From: Rich Felker @ 2018-03-27 23:58 UTC (permalink / raw)
To: Theodore Y. Ts'o, Ilya Smith, Michal Hocko, Matthew Wilcox,
rth, ink, mattst88, vgupta, linux, tony.luck, fenghua.yu, ralf,
jejb, Helge Deller, benh, paulus, mpe, schwidefsky,
heiko.carstens, ysato, davem, tglx, mingo, hpa, x86, nyc, viro,
arnd, gregkh, deepa.kernel, Hugh Dickins, kstewart, pombredanne,
Andrew Morton, steve.capper, punit.agrawal, aneesh.kumar, npiggin,
Kees Cook, bhsharma, riel, nitin.m.gupta, Kirill A. Shutemov,
Dan Williams, Jan Kara, ross.zwisler, Jerome Glisse,
Andrea Arcangeli, Oleg Nesterov, linux-alpha, LKML,
linux-snps-arc, linux-ia64, linux-metag, linux-mips, linux-parisc,
linuxppc-dev, linux-s390, linux-sh, sparclinux, Linux-MM
In-Reply-To: <20180327221635.GA3790@thunk.org>
On Tue, Mar 27, 2018 at 06:16:35PM -0400, Theodore Y. Ts'o wrote:
> On Tue, Mar 27, 2018 at 04:51:08PM +0300, Ilya Smith wrote:
> > > /dev/[u]random is not sufficient?
> >
> > Using /dev/[u]random makes 3 syscalls - open, read, close. This is a performance
> > issue.
>
> You may want to take a look at the getrandom(2) system call, which is
> the recommended way getting secure random numbers from the kernel.
Yes, while opening /dev/urandom is not acceptable due to needing an
fd, getrandom and existing fallbacks for it have this covered if
needed.
> > > Well, I am pretty sure userspace can implement proper free ranges
> > > tracking…
> >
> > I think we need to know what libc developers will say on implementing ASLR in
> > user-mode. I am pretty sure they will say ‘nether’ or ‘some-day’. And problem
> > of ASLR will stay forever.
>
> Why can't you send patches to the libc developers?
I can tell you right now that any patch submitted for musl that
depended on trying to duplicate knowledge of the entire virtual
address space layout in userspace as part of mmap would be rejected,
and I would recommend glibc do the same.
Not only does it vastly increase complexity; it also has all sorts of
failure modes (fd exhastion, etc.) which would either introduce new
and unwanted ways for mmap to fail, or would force fallback to the
normal (no extra randomization) strategy under conditions an attacker
could potentially control, defeating the whole purpose. It would also
potentially make it easier for an attacker to examine the vm layout
for attacks, since it would be recorded in userspace.
There's also the issue of preserving AS-safety of mmap. POSIX does not
actually require mmap to be AS-safe, and on musl munmap is not fully
AS-safe anyway because of some obscure issues it compensates for, but
we may be able to make it AS-safe (this is a low-priority open issue).
If mmap were manipulating data structures representing the vm space in
userspace, though, the only way to make it anywhere near AS-safe would
be to block all signals and take a lock every time mmap or munmap is
called. This would significantly increase the cost of each call,
especially now that meltdown/spectre mitigations have greatly
increased the overhead of each syscall.
Overall, asking userspace to take a lead role in management of process
vm space is a radical change in the split of what user and kernel are
responsible for, and it really does not make sense as part of a
dubious hardening measure. Something this big would need to be really
well-motivated.
Rich
^ permalink raw reply
* Re: [RFC PATCH v2 0/2] Randomization of address chosen by mmap.
From: Kees Cook @ 2018-03-27 23:57 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Ilya Smith, Michal Hocko, Richard Henderson, ink, mattst88,
Vineet Gupta, Russell King, Tony Luck, Fenghua Yu, Ralf Baechle,
James E.J. Bottomley, Helge Deller, Benjamin Herrenschmidt,
Paul Mackerras, Michael Ellerman, Martin Schwidefsky,
Heiko Carstens, Yoshinori Sato, Rich Felker, David S. Miller,
Thomas Gleixner, Ingo Molnar, H. Peter Anvin, X86 ML, nyc,
Al Viro, Arnd Bergmann, Greg KH, Deepa Dinamani, Hugh Dickins,
Kate Stewart, Philippe Ombredanne, Andrew Morton, Steve Capper,
Punit Agrawal, Aneesh Kumar K.V, Nick Piggin, Bhupesh Sharma,
Rik van Riel, nitin.m.gupta, Kirill A. Shutemov, Dan Williams,
Jan Kara, Ross Zwisler, Jerome Glisse, Andrea Arcangeli,
Oleg Nesterov, linux-alpha, LKML, linux-snps-arc, linux-ia64,
linux-metag, Linux MIPS Mailing List, linux-parisc, PowerPC,
linux-s390, linux-sh, sparclinux, Linux-MM
In-Reply-To: <20180327234904.GA27734@bombadil.infradead.org>
On Tue, Mar 27, 2018 at 4:49 PM, Matthew Wilcox <willy@infradead.org> wrote:
> On Tue, Mar 27, 2018 at 03:53:53PM -0700, Kees Cook wrote:
>> I agree: pushing this off to libc leaves a lot of things unprotected.
>> I think this should live in the kernel. The question I have is about
>> making it maintainable/readable/etc.
>>
>> The state-of-the-art for ASLR is moving to finer granularity (over
>> just base-address offset), so I'd really like to see this supported in
>> the kernel. We'll be getting there for other things in the future, and
>> I'd like to have a working production example for researchers to
>> study, etc.
>
> One thing we need is to limit the fragmentation of this approach.
> Even on 64-bit systems, we can easily get into a situation where there isn't
> space to map a contiguous terabyte.
FWIW, I wouldn't expect normal systems to use this. I am curious about
fragmentation vs entropy though. Are workloads with a mis of lots of
tiny allocations and TB-allocations? AIUI, glibc uses larger mmap()
regions for handling tiny mallocs().
-Kees
--
Kees Cook
Pixel Security
^ permalink raw reply
* Re: [RFC PATCH v2 0/2] Randomization of address chosen by mmap.
From: Matthew Wilcox @ 2018-03-27 23:49 UTC (permalink / raw)
To: Kees Cook
Cc: Ilya Smith, Michal Hocko, Richard Henderson, ink, mattst88,
Vineet Gupta, Russell King, Tony Luck, Fenghua Yu, Ralf Baechle,
James E.J. Bottomley, Helge Deller, Benjamin Herrenschmidt,
Paul Mackerras, Michael Ellerman, Martin Schwidefsky,
Heiko Carstens, Yoshinori Sato, Rich Felker, David S. Miller,
Thomas Gleixner, Ingo Molnar, H. Peter Anvin, X86 ML, nyc,
Al Viro, Arnd Bergmann, Greg KH, Deepa Dinamani, Hugh Dickins,
Kate Stewart, Philippe Ombredanne, Andrew Morton, Steve Capper,
Punit Agrawal, Aneesh Kumar K.V, Nick Piggin, Bhupesh Sharma,
Rik van Riel, nitin.m.gupta, Kirill A. Shutemov, Dan Williams,
Jan Kara, Ross Zwisler, Jerome Glisse, Andrea Arcangeli,
Oleg Nesterov, linux-alpha, LKML, linux-snps-arc, linux-ia64,
linux-metag, Linux MIPS Mailing List, linux-parisc, PowerPC,
linux-s390, linux-sh, sparclinux, Linux-MM
In-Reply-To: <CAGXu5j+XXufprMaJ9GbHxD3mZ7iqUuu60-tTMC6wo2x1puYzMQ@mail.gmail.com>
On Tue, Mar 27, 2018 at 03:53:53PM -0700, Kees Cook wrote:
> I agree: pushing this off to libc leaves a lot of things unprotected.
> I think this should live in the kernel. The question I have is about
> making it maintainable/readable/etc.
>
> The state-of-the-art for ASLR is moving to finer granularity (over
> just base-address offset), so I'd really like to see this supported in
> the kernel. We'll be getting there for other things in the future, and
> I'd like to have a working production example for researchers to
> study, etc.
One thing we need is to limit the fragmentation of this approach.
Even on 64-bit systems, we can easily get into a situation where there isn't
space to map a contiguous terabyte.
^ permalink raw reply
* Re: RFC on writel and writel_relaxed
From: Benjamin Herrenschmidt @ 2018-03-27 23:43 UTC (permalink / raw)
To: Alexander Duyck
Cc: Sinan Kaya, Arnd Bergmann, Jason Gunthorpe, David Laight, Oliver,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT),
linux-rdma@vger.kernel.org, Will Deacon, Paul E. McKenney,
netdev@vger.kernel.org
In-Reply-To: <CAKgT0UfZKBTYPCqWtBF=N5TPFrbsv0ZU+2GHMqF3J4BM9Hw4yw@mail.gmail.com>
On Tue, 2018-03-27 at 14:54 -0700, Alexander Duyck wrote:
> On Tue, Mar 27, 2018 at 2:35 PM, Benjamin Herrenschmidt
> <benh@kernel.crashing.org> wrote:
> > On Tue, 2018-03-27 at 10:46 -0400, Sinan Kaya wrote:
> > > combined buffers.
> > >
> > > Alex:
> > > "Don't bother. I can tell you right now that for x86 you have to have a
> > > wmb() before the writel().
> >
> > No, this isn't the semantics of writel. You shouldn't need it unless
> > something changed and we need to revisit our complete understanding of
> > *all* MMIO accessor semantics.
>
> The issue seems to be that there have been two different ways of
> dealing with this. There has historically been a number of different
> drivers that have been carrying this wmb() workaround since something
> like 2002. I get that the semantics for writel might have changed
> since then, but those of us who already have the wmb() in our drivers
> will be very wary of anyone wanting to go through and remove them
> since writel is supposed to be "good enough". I would much rather err
> on the side of caution here.
>
> I view the wmb() + writel_relaxed() as more of a driver owning and
> handling this itself. Besides in the Intel Ethernet driver case it is
> better performance as our wmb() placement for us also provides a
> secondary barrier so we don't need to add a separate smp_wmb() to deal
> with a potential race we have with the Tx cleanup.
>
> > At least for UC space, it has always been accepted (and enforced) that
> > writel would not require any other barrier to order vs. previous stores
> > to memory.
>
> So the one thing I would question here is if this is UC vs UC or if
> this extends to other types as well? So for x86 we could find
> references to Write Combining being flushed by a write to UC memory,
> however I have yet to find a clear explanation of what a write to UC
> does to WB.
Well, this is the standard write memory + trigger DMA case, the one
specific case for which Linus was adamant we don't need another barrier
back then ...
> My personal inclination would be to err on the side of
> caution.
Which means writel_relaxed is now pointless ?
We need clear semantics here. In this case the "side of caution" means
we are randomly doing things not understanding what really happens and
that makes me *more* nervous.
> I just don't want us going through and removing the wmb()
> calls because it "should" work. I would want to know for certain it
> will work.
We need to know for certain anyway. Otherwise, all the drivers that do
not have wmb's are potentially broken.
So I dont agree with the status quo.
We need to establish precisely what x86 does, decide what we want the
semantic of writel to be, and implement things accordingly.
Ben.
^ permalink raw reply
* Re: [RFC PATCH v2 0/2] Randomization of address chosen by mmap.
From: Theodore Y. Ts'o @ 2018-03-27 22:16 UTC (permalink / raw)
To: Ilya Smith
Cc: Michal Hocko, Matthew Wilcox, rth, ink, mattst88, vgupta, linux,
tony.luck, fenghua.yu, ralf, jejb, Helge Deller, benh, paulus,
mpe, schwidefsky, heiko.carstens, ysato, dalias, davem, tglx,
mingo, hpa, x86, nyc, viro, arnd, gregkh, deepa.kernel,
Hugh Dickins, kstewart, pombredanne, Andrew Morton, steve.capper,
punit.agrawal, aneesh.kumar, npiggin, Kees Cook, bhsharma, riel,
nitin.m.gupta, Kirill A. Shutemov, Dan Williams, Jan Kara,
ross.zwisler, Jerome Glisse, Andrea Arcangeli, Oleg Nesterov,
linux-alpha, LKML, linux-snps-arc, linux-ia64, linux-metag,
linux-mips, linux-parisc, linuxppc-dev, linux-s390, linux-sh,
sparclinux, Linux-MM
In-Reply-To: <0549F29C-12FC-4401-9E85-A430BC11DA78@gmail.com>
On Tue, Mar 27, 2018 at 04:51:08PM +0300, Ilya Smith wrote:
> > /dev/[u]random is not sufficient?
>
> Using /dev/[u]random makes 3 syscalls - open, read, close. This is a performance
> issue.
You may want to take a look at the getrandom(2) system call, which is
the recommended way getting secure random numbers from the kernel.
> > Well, I am pretty sure userspace can implement proper free ranges
> > tracking…
>
> I think we need to know what libc developers will say on implementing ASLR in
> user-mode. I am pretty sure they will say ‘nether’ or ‘some-day’. And problem
> of ASLR will stay forever.
Why can't you send patches to the libc developers?
Regards,
- Ted
^ permalink raw reply
* Re: [RFC PATCH v2 0/2] Randomization of address chosen by mmap.
From: Kees Cook @ 2018-03-27 22:53 UTC (permalink / raw)
To: Ilya Smith
Cc: Michal Hocko, Matthew Wilcox, Richard Henderson, ink, mattst88,
Vineet Gupta, Russell King, Tony Luck, Fenghua Yu, Ralf Baechle,
James E.J. Bottomley, Helge Deller, Benjamin Herrenschmidt,
Paul Mackerras, Michael Ellerman, Martin Schwidefsky,
Heiko Carstens, Yoshinori Sato, Rich Felker, David S. Miller,
Thomas Gleixner, Ingo Molnar, H. Peter Anvin, X86 ML, nyc,
Al Viro, Arnd Bergmann, Greg KH, Deepa Dinamani, Hugh Dickins,
Kate Stewart, Philippe Ombredanne, Andrew Morton, Steve Capper,
Punit Agrawal, Aneesh Kumar K.V, Nick Piggin, Bhupesh Sharma,
Rik van Riel, nitin.m.gupta, Kirill A. Shutemov, Dan Williams,
Jan Kara, Ross Zwisler, Jerome Glisse, Andrea Arcangeli,
Oleg Nesterov, linux-alpha, LKML, linux-snps-arc, linux-ia64,
linux-metag, Linux MIPS Mailing List, linux-parisc, PowerPC,
linux-s390, linux-sh, sparclinux, Linux-MM
In-Reply-To: <0549F29C-12FC-4401-9E85-A430BC11DA78@gmail.com>
On Tue, Mar 27, 2018 at 6:51 AM, Ilya Smith <blackzert@gmail.com> wrote:
>
>> On 27 Mar 2018, at 10:24, Michal Hocko <mhocko@kernel.org> wrote:
>>
>> On Mon 26-03-18 22:45:31, Ilya Smith wrote:
>>>
>>>> On 26 Mar 2018, at 11:46, Michal Hocko <mhocko@kernel.org> wrote:
>>>>
>>>> On Fri 23-03-18 20:55:49, Ilya Smith wrote:
>>>>>
>>>>>> On 23 Mar 2018, at 15:48, Matthew Wilcox <willy@infradead.org> wrote=
:
>>>>>>
>>>>>> On Thu, Mar 22, 2018 at 07:36:36PM +0300, Ilya Smith wrote:
>>>>>>> Current implementation doesn't randomize address returned by mmap.
>>>>>>> All the entropy ends with choosing mmap_base_addr at the process
>>>>>>> creation. After that mmap build very predictable layout of address
>>>>>>> space. It allows to bypass ASLR in many cases. This patch make
>>>>>>> randomization of address on any mmap call.
>>>>>>
>>>>>> Why should this be done in the kernel rather than libc? libc is per=
fectly
>>>>>> capable of specifying random numbers in the first argument of mmap.
>>>>> Well, there is following reasons:
>>>>> 1. It should be done in any libc implementation, what is not possible=
IMO;
>>>>
>>>> Is this really so helpful?
>>>
>>> Yes, ASLR is one of very important mitigation techniques which are real=
ly used
>>> to protect applications. If there is no ASLR, it is very easy to exploi=
t
>>> vulnerable application and compromise the system. We can=E2=80=99t just=
fix all the
>>> vulnerabilities right now, thats why we have mitigations - techniques w=
hich are
>>> makes exploitation more hard or impossible in some cases.
>>>
>>> Thats why it is helpful.
>>
>> I am not questioning ASLR in general. I am asking whether we really need
>> per mmap ASLR in general. I can imagine that some environments want to
>> pay the additional price and other side effects, but considering this
>> can be achieved by libc, why to add more code to the kernel?
>
> I believe this is the only one right place for it. Adding these 200+ line=
s of
> code we give this feature for any user - on desktop, on server, on IoT de=
vice,
> on SCADA, etc. But if only glibc will implement =E2=80=98user-mode-aslr=
=E2=80=99 IoT and SCADA
> devices will never get it.
I agree: pushing this off to libc leaves a lot of things unprotected.
I think this should live in the kernel. The question I have is about
making it maintainable/readable/etc.
The state-of-the-art for ASLR is moving to finer granularity (over
just base-address offset), so I'd really like to see this supported in
the kernel. We'll be getting there for other things in the future, and
I'd like to have a working production example for researchers to
study, etc.
-Kees
--=20
Kees Cook
Pixel Security
^ permalink raw reply
* Re: RFC on writel and writel_relaxed
From: Sinan Kaya @ 2018-03-27 22:35 UTC (permalink / raw)
To: Alexander Duyck, Benjamin Herrenschmidt
Cc: Arnd Bergmann, Jason Gunthorpe, David Laight, Oliver,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT),
linux-rdma@vger.kernel.org, Will Deacon, Paul E. McKenney,
netdev@vger.kernel.org
In-Reply-To: <CAKgT0UfZKBTYPCqWtBF=N5TPFrbsv0ZU+2GHMqF3J4BM9Hw4yw@mail.gmail.com>
On 3/27/2018 5:54 PM, Alexander Duyck wrote:
> I view the wmb() + writel_relaxed() as more of a driver owning and
> handling this itself. Besides in the Intel Ethernet driver case it is
> better performance as our wmb() placement for us also provides a
> secondary barrier so we don't need to add a separate smp_wmb() to deal
> with a potential race we have with the Tx cleanup.
Thanks for the reminder.
I forgot about the double barrier optimization. wmb() + writel_relaxed()
seems to be the best option for Intel network drivers at this moment.
Otherwise, we'll have to remove wmb() and throw in smp barriers there
like you mentioned.
I'll leave the changes in the Intel drivers alone.
--
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply
* Re: [PATCH v9 09/24] mm: protect mremap() against SPF hanlder
From: David Rientjes @ 2018-03-27 22:12 UTC (permalink / raw)
To: Laurent Dufour
Cc: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack,
Matthew Wilcox, benh, mpe, paulus, Thomas Gleixner, Ingo Molnar,
hpa, Will Deacon, Sergey Senozhatsky, Andrea Arcangeli,
Alexei Starovoitov, kemi.wang, sergey.senozhatsky.work,
Daniel Jordan, linux-kernel, linux-mm, haren, khandual, npiggin,
bsingharora, Tim Chen, linuxppc-dev, x86
In-Reply-To: <1520963994-28477-10-git-send-email-ldufour@linux.vnet.ibm.com>
On Tue, 13 Mar 2018, Laurent Dufour wrote:
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 88042d843668..ef6ef0627090 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2189,16 +2189,24 @@ void anon_vma_interval_tree_verify(struct anon_vma_chain *node);
> extern int __vm_enough_memory(struct mm_struct *mm, long pages, int cap_sys_admin);
> extern int __vma_adjust(struct vm_area_struct *vma, unsigned long start,
> unsigned long end, pgoff_t pgoff, struct vm_area_struct *insert,
> - struct vm_area_struct *expand);
> + struct vm_area_struct *expand, bool keep_locked);
> static inline int vma_adjust(struct vm_area_struct *vma, unsigned long start,
> unsigned long end, pgoff_t pgoff, struct vm_area_struct *insert)
> {
> - return __vma_adjust(vma, start, end, pgoff, insert, NULL);
> + return __vma_adjust(vma, start, end, pgoff, insert, NULL, false);
> }
> -extern struct vm_area_struct *vma_merge(struct mm_struct *,
> +extern struct vm_area_struct *__vma_merge(struct mm_struct *,
> struct vm_area_struct *prev, unsigned long addr, unsigned long end,
> unsigned long vm_flags, struct anon_vma *, struct file *, pgoff_t,
> - struct mempolicy *, struct vm_userfaultfd_ctx);
> + struct mempolicy *, struct vm_userfaultfd_ctx, bool keep_locked);
> +static inline struct vm_area_struct *vma_merge(struct mm_struct *vma,
> + struct vm_area_struct *prev, unsigned long addr, unsigned long end,
> + unsigned long vm_flags, struct anon_vma *anon, struct file *file,
> + pgoff_t off, struct mempolicy *pol, struct vm_userfaultfd_ctx uff)
> +{
> + return __vma_merge(vma, prev, addr, end, vm_flags, anon, file, off,
> + pol, uff, false);
> +}
The first formal to vma_merge() is an mm, not a vma.
This area could use an uncluttering.
> extern struct anon_vma *find_mergeable_anon_vma(struct vm_area_struct *);
> extern int __split_vma(struct mm_struct *, struct vm_area_struct *,
> unsigned long addr, int new_below);
> diff --git a/mm/mmap.c b/mm/mmap.c
> index d6533cb85213..ac32b577a0c9 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -684,7 +684,7 @@ static inline void __vma_unlink_prev(struct mm_struct *mm,
> */
> int __vma_adjust(struct vm_area_struct *vma, unsigned long start,
> unsigned long end, pgoff_t pgoff, struct vm_area_struct *insert,
> - struct vm_area_struct *expand)
> + struct vm_area_struct *expand, bool keep_locked)
> {
> struct mm_struct *mm = vma->vm_mm;
> struct vm_area_struct *next = vma->vm_next, *orig_vma = vma;
> @@ -996,7 +996,8 @@ int __vma_adjust(struct vm_area_struct *vma, unsigned long start,
>
> if (next && next != vma)
> vm_raw_write_end(next);
> - vm_raw_write_end(vma);
> + if (!keep_locked)
> + vm_raw_write_end(vma);
>
> validate_mm(mm);
>
This will require a fixup for the following patch where a retval from
anon_vma_close() can also return without vma locked even though
keep_locked == true.
How does vma_merge() handle that error wrt vm_raw_write_begin(vma)?
> @@ -1132,12 +1133,13 @@ can_vma_merge_after(struct vm_area_struct *vma, unsigned long vm_flags,
> * parameter) may establish ptes with the wrong permissions of NNNN
> * instead of the right permissions of XXXX.
> */
> -struct vm_area_struct *vma_merge(struct mm_struct *mm,
> +struct vm_area_struct *__vma_merge(struct mm_struct *mm,
> struct vm_area_struct *prev, unsigned long addr,
> unsigned long end, unsigned long vm_flags,
> struct anon_vma *anon_vma, struct file *file,
> pgoff_t pgoff, struct mempolicy *policy,
> - struct vm_userfaultfd_ctx vm_userfaultfd_ctx)
> + struct vm_userfaultfd_ctx vm_userfaultfd_ctx,
> + bool keep_locked)
> {
> pgoff_t pglen = (end - addr) >> PAGE_SHIFT;
> struct vm_area_struct *area, *next;
> @@ -1185,10 +1187,11 @@ struct vm_area_struct *vma_merge(struct mm_struct *mm,
> /* cases 1, 6 */
> err = __vma_adjust(prev, prev->vm_start,
> next->vm_end, prev->vm_pgoff, NULL,
> - prev);
> + prev, keep_locked);
> } else /* cases 2, 5, 7 */
> err = __vma_adjust(prev, prev->vm_start,
> - end, prev->vm_pgoff, NULL, prev);
> + end, prev->vm_pgoff, NULL, prev,
> + keep_locked);
> if (err)
> return NULL;
> khugepaged_enter_vma_merge(prev, vm_flags);
> @@ -1205,10 +1208,12 @@ struct vm_area_struct *vma_merge(struct mm_struct *mm,
> vm_userfaultfd_ctx)) {
> if (prev && addr < prev->vm_end) /* case 4 */
> err = __vma_adjust(prev, prev->vm_start,
> - addr, prev->vm_pgoff, NULL, next);
> + addr, prev->vm_pgoff, NULL, next,
> + keep_locked);
> else { /* cases 3, 8 */
> err = __vma_adjust(area, addr, next->vm_end,
> - next->vm_pgoff - pglen, NULL, next);
> + next->vm_pgoff - pglen, NULL, next,
> + keep_locked);
> /*
> * In case 3 area is already equal to next and
> * this is a noop, but in case 8 "area" has
> @@ -3163,9 +3168,20 @@ struct vm_area_struct *copy_vma(struct vm_area_struct **vmap,
>
> if (find_vma_links(mm, addr, addr + len, &prev, &rb_link, &rb_parent))
> return NULL; /* should never get here */
> - new_vma = vma_merge(mm, prev, addr, addr + len, vma->vm_flags,
> - vma->anon_vma, vma->vm_file, pgoff, vma_policy(vma),
> - vma->vm_userfaultfd_ctx);
> +
> + /* There is 3 cases to manage here in
> + * AAAA AAAA AAAA AAAA
> + * PPPP.... PPPP......NNNN PPPP....NNNN PP........NN
> + * PPPPPPPP(A) PPPP..NNNNNNNN(B) PPPPPPPPPPPP(1) NULL
> + * PPPPPPPPNNNN(2)
> + * PPPPNNNNNNNN(3)
> + *
> + * new_vma == prev in case A,1,2
> + * new_vma == next in case B,3
> + */
Interleaved tabs and whitespace.
> + new_vma = __vma_merge(mm, prev, addr, addr + len, vma->vm_flags,
> + vma->anon_vma, vma->vm_file, pgoff,
> + vma_policy(vma), vma->vm_userfaultfd_ctx, true);
> if (new_vma) {
> /*
> * Source vma may have been merged into new_vma
> @@ -3205,6 +3221,15 @@ struct vm_area_struct *copy_vma(struct vm_area_struct **vmap,
> get_file(new_vma->vm_file);
> if (new_vma->vm_ops && new_vma->vm_ops->open)
> new_vma->vm_ops->open(new_vma);
> + /*
> + * As the VMA is linked right now, it may be hit by the
> + * speculative page fault handler. But we don't want it to
> + * to start mapping page in this area until the caller has
> + * potentially move the pte from the moved VMA. To prevent
> + * that we protect it right now, and let the caller unprotect
> + * it once the move is done.
> + */
> + vm_raw_write_begin(new_vma);
> vma_link(mm, new_vma, prev, rb_link, rb_parent);
> *need_rmap_locks = false;
> }
> diff --git a/mm/mremap.c b/mm/mremap.c
> index 049470aa1e3e..8ed1a1d6eaed 100644
> --- a/mm/mremap.c
> +++ b/mm/mremap.c
> @@ -302,6 +302,14 @@ static unsigned long move_vma(struct vm_area_struct *vma,
> if (!new_vma)
> return -ENOMEM;
>
> + /* new_vma is returned protected by copy_vma, to prevent speculative
> + * page fault to be done in the destination area before we move the pte.
> + * Now, we must also protect the source VMA since we don't want pages
> + * to be mapped in our back while we are copying the PTEs.
> + */
> + if (vma != new_vma)
> + vm_raw_write_begin(vma);
> +
> moved_len = move_page_tables(vma, old_addr, new_vma, new_addr, old_len,
> need_rmap_locks);
> if (moved_len < old_len) {
> @@ -318,6 +326,8 @@ static unsigned long move_vma(struct vm_area_struct *vma,
> */
> move_page_tables(new_vma, new_addr, vma, old_addr, moved_len,
> true);
> + if (vma != new_vma)
> + vm_raw_write_end(vma);
> vma = new_vma;
> old_len = new_len;
> old_addr = new_addr;
> @@ -326,7 +336,10 @@ static unsigned long move_vma(struct vm_area_struct *vma,
> mremap_userfaultfd_prep(new_vma, uf);
> arch_remap(mm, old_addr, old_addr + old_len,
> new_addr, new_addr + new_len);
> + if (vma != new_vma)
> + vm_raw_write_end(vma);
> }
> + vm_raw_write_end(new_vma);
Just do
vm_raw_write_end(vma);
vm_raw_write_end(new_vma);
here.
^ permalink raw reply
* Re: [PATCH v9 08/24] mm: Protect VMA modifications using VMA sequence count
From: David Rientjes @ 2018-03-27 21:57 UTC (permalink / raw)
To: Laurent Dufour
Cc: paulmck, peterz, Andrew Morton, kirill, ak, mhocko, dave, jack,
Matthew Wilcox, benh, mpe, paulus, Thomas Gleixner, Ingo Molnar,
hpa, Will Deacon, Sergey Senozhatsky, Andrea Arcangeli,
Alexei Starovoitov, kemi.wang, sergey.senozhatsky.work,
Daniel Jordan, linux-kernel, linux-mm, haren, khandual, npiggin,
bsingharora, Tim Chen, linuxppc-dev, x86
In-Reply-To: <1520963994-28477-9-git-send-email-ldufour@linux.vnet.ibm.com>
On Tue, 13 Mar 2018, Laurent Dufour wrote:
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 5898255d0aeb..d6533cb85213 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -847,17 +847,18 @@ int __vma_adjust(struct vm_area_struct *vma, unsigned long start,
> }
>
> if (start != vma->vm_start) {
> - vma->vm_start = start;
> + WRITE_ONCE(vma->vm_start, start);
> start_changed = true;
> }
> if (end != vma->vm_end) {
> - vma->vm_end = end;
> + WRITE_ONCE(vma->vm_end, end);
> end_changed = true;
> }
> - vma->vm_pgoff = pgoff;
> + WRITE_ONCE(vma->vm_pgoff, pgoff);
> if (adjust_next) {
> - next->vm_start += adjust_next << PAGE_SHIFT;
> - next->vm_pgoff += adjust_next;
> + WRITE_ONCE(next->vm_start,
> + next->vm_start + (adjust_next << PAGE_SHIFT));
> + WRITE_ONCE(next->vm_pgoff, next->vm_pgoff + adjust_next);
> }
>
> if (root) {
> @@ -1781,6 +1782,7 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
> out:
> perf_event_mmap(vma);
>
> + vm_write_begin(vma);
> vm_stat_account(mm, vm_flags, len >> PAGE_SHIFT);
> if (vm_flags & VM_LOCKED) {
> if (!((vm_flags & VM_SPECIAL) || is_vm_hugetlb_page(vma) ||
> @@ -1803,6 +1805,7 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
> vma->vm_flags |= VM_SOFTDIRTY;
>
> vma_set_page_prot(vma);
> + vm_write_end(vma);
>
> return addr;
>
Shouldn't this also protect vma->vm_flags?
diff --git a/mm/mmap.c b/mm/mmap.c
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1796,7 +1796,8 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
vma == get_gate_vma(current->mm)))
mm->locked_vm += (len >> PAGE_SHIFT);
else
- vma->vm_flags &= VM_LOCKED_CLEAR_MASK;
+ WRITE_ONCE(vma->vm_flags,
+ vma->vm_flags & VM_LOCKED_CLEAR_MASK);
}
if (file)
@@ -1809,7 +1810,7 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
* then new mapped in-place (which must be aimed as
* a completely new data area).
*/
- vma->vm_flags |= VM_SOFTDIRTY;
+ WRITE_ONCE(vma->vm_flags, vma->vm_flags | VM_SOFTDIRTY);
vma_set_page_prot(vma);
vm_write_end(vma);
^ permalink raw reply
* Re: RFC on writel and writel_relaxed
From: Alexander Duyck @ 2018-03-27 21:54 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Sinan Kaya, Arnd Bergmann, Jason Gunthorpe, David Laight, Oliver,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT),
linux-rdma@vger.kernel.org, Will Deacon, Paul E. McKenney,
netdev@vger.kernel.org
On Tue, Mar 27, 2018 at 2:35 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Tue, 2018-03-27 at 10:46 -0400, Sinan Kaya wrote:
>> combined buffers.
>>
>> Alex:
>> "Don't bother. I can tell you right now that for x86 you have to have a
>> wmb() before the writel().
>
> No, this isn't the semantics of writel. You shouldn't need it unless
> something changed and we need to revisit our complete understanding of
> *all* MMIO accessor semantics.
The issue seems to be that there have been two different ways of
dealing with this. There has historically been a number of different
drivers that have been carrying this wmb() workaround since something
like 2002. I get that the semantics for writel might have changed
since then, but those of us who already have the wmb() in our drivers
will be very wary of anyone wanting to go through and remove them
since writel is supposed to be "good enough". I would much rather err
on the side of caution here.
I view the wmb() + writel_relaxed() as more of a driver owning and
handling this itself. Besides in the Intel Ethernet driver case it is
better performance as our wmb() placement for us also provides a
secondary barrier so we don't need to add a separate smp_wmb() to deal
with a potential race we have with the Tx cleanup.
> At least for UC space, it has always been accepted (and enforced) that
> writel would not require any other barrier to order vs. previous stores
> to memory.
So the one thing I would question here is if this is UC vs UC or if
this extends to other types as well? So for x86 we could find
references to Write Combining being flushed by a write to UC memory,
however I have yet to find a clear explanation of what a write to UC
does to WB. My personal inclination would be to err on the side of
caution. I just don't want us going through and removing the wmb()
calls because it "should" work. I would want to know for certain it
will work.
- Alex
^ permalink raw reply
* Re: [PATCH v9 08/24] mm: Protect VMA modifications using VMA sequence count
From: David Rientjes @ 2018-03-27 21:45 UTC (permalink / raw)
To: Laurent Dufour
Cc: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack,
Matthew Wilcox, benh, mpe, paulus, Thomas Gleixner, Ingo Molnar,
hpa, Will Deacon, Sergey Senozhatsky, Andrea Arcangeli,
Alexei Starovoitov, kemi.wang, sergey.senozhatsky.work,
Daniel Jordan, linux-kernel, linux-mm, haren, khandual, npiggin,
bsingharora, Tim Chen, linuxppc-dev, x86
In-Reply-To: <1520963994-28477-9-git-send-email-ldufour@linux.vnet.ibm.com>
On Tue, 13 Mar 2018, Laurent Dufour wrote:
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index 65ae54659833..a2d9c87b7b0b 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -1136,8 +1136,11 @@ static ssize_t clear_refs_write(struct file *file, const char __user *buf,
> goto out_mm;
> }
> for (vma = mm->mmap; vma; vma = vma->vm_next) {
> - vma->vm_flags &= ~VM_SOFTDIRTY;
> + vm_write_begin(vma);
> + WRITE_ONCE(vma->vm_flags,
> + vma->vm_flags & ~VM_SOFTDIRTY);
> vma_set_page_prot(vma);
> + vm_write_end(vma);
> }
> downgrade_write(&mm->mmap_sem);
> break;
> diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
> index cec550c8468f..b8212ba17695 100644
> --- a/fs/userfaultfd.c
> +++ b/fs/userfaultfd.c
> @@ -659,8 +659,11 @@ int dup_userfaultfd(struct vm_area_struct *vma, struct list_head *fcs)
>
> octx = vma->vm_userfaultfd_ctx.ctx;
> if (!octx || !(octx->features & UFFD_FEATURE_EVENT_FORK)) {
> + vm_write_begin(vma);
> vma->vm_userfaultfd_ctx = NULL_VM_UFFD_CTX;
> - vma->vm_flags &= ~(VM_UFFD_WP | VM_UFFD_MISSING);
> + WRITE_ONCE(vma->vm_flags,
> + vma->vm_flags & ~(VM_UFFD_WP | VM_UFFD_MISSING));
> + vm_write_end(vma);
> return 0;
> }
>
In several locations in this patch vm_write_begin(vma) ->
vm_write_end(vma) is nesting things other than vma->vm_flags,
vma->vm_policy, etc. I think it's better to do vm_write_end(vma) as soon
as the members that the seqcount protects are modified. In other words,
this isn't offering protection for vma->vm_userfaultfd_ctx. There are
several examples of this in the patch.
> @@ -885,8 +888,10 @@ static int userfaultfd_release(struct inode *inode, struct file *file)
> vma = prev;
> else
> prev = vma;
> - vma->vm_flags = new_flags;
> + vm_write_begin(vma);
> + WRITE_ONCE(vma->vm_flags, new_flags);
> vma->vm_userfaultfd_ctx = NULL_VM_UFFD_CTX;
> + vm_write_end(vma);
> }
> up_write(&mm->mmap_sem);
> mmput(mm);
> @@ -1434,8 +1439,10 @@ static int userfaultfd_register(struct userfaultfd_ctx *ctx,
> * the next vma was merged into the current one and
> * the current one has not been updated yet.
> */
> - vma->vm_flags = new_flags;
> + vm_write_begin(vma);
> + WRITE_ONCE(vma->vm_flags, new_flags);
> vma->vm_userfaultfd_ctx.ctx = ctx;
> + vm_write_end(vma);
>
> skip:
> prev = vma;
> @@ -1592,8 +1599,10 @@ static int userfaultfd_unregister(struct userfaultfd_ctx *ctx,
> * the next vma was merged into the current one and
> * the current one has not been updated yet.
> */
> - vma->vm_flags = new_flags;
> + vm_write_begin(vma);
> + WRITE_ONCE(vma->vm_flags, new_flags);
> vma->vm_userfaultfd_ctx = NULL_VM_UFFD_CTX;
> + vm_write_end(vma);
>
> skip:
> prev = vma;
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index b7e2268dfc9a..32314e9e48dd 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -1006,6 +1006,7 @@ static void collapse_huge_page(struct mm_struct *mm,
> if (mm_find_pmd(mm, address) != pmd)
> goto out;
>
> + vm_write_begin(vma);
> anon_vma_lock_write(vma->anon_vma);
>
> pte = pte_offset_map(pmd, address);
> @@ -1041,6 +1042,7 @@ static void collapse_huge_page(struct mm_struct *mm,
> pmd_populate(mm, pmd, pmd_pgtable(_pmd));
> spin_unlock(pmd_ptl);
> anon_vma_unlock_write(vma->anon_vma);
> + vm_write_end(vma);
> result = SCAN_FAIL;
> goto out;
> }
> @@ -1075,6 +1077,7 @@ static void collapse_huge_page(struct mm_struct *mm,
> set_pmd_at(mm, address, pmd, _pmd);
> update_mmu_cache_pmd(vma, address, pmd);
> spin_unlock(pmd_ptl);
> + vm_write_end(vma);
>
> *hpage = NULL;
>
> diff --git a/mm/madvise.c b/mm/madvise.c
> index 4d3c922ea1a1..e328f7ab5942 100644
> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -184,7 +184,9 @@ static long madvise_behavior(struct vm_area_struct *vma,
> /*
> * vm_flags is protected by the mmap_sem held in write mode.
> */
> - vma->vm_flags = new_flags;
> + vm_write_begin(vma);
> + WRITE_ONCE(vma->vm_flags, new_flags);
> + vm_write_end(vma);
> out:
> return error;
> }
> @@ -450,9 +452,11 @@ static void madvise_free_page_range(struct mmu_gather *tlb,
> .private = tlb,
> };
>
> + vm_write_begin(vma);
> tlb_start_vma(tlb, vma);
> walk_page_range(addr, end, &free_walk);
> tlb_end_vma(tlb, vma);
> + vm_write_end(vma);
> }
>
> static int madvise_free_single_vma(struct vm_area_struct *vma,
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index e0e706f0b34e..2632c6f93b63 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -380,8 +380,11 @@ void mpol_rebind_mm(struct mm_struct *mm, nodemask_t *new)
> struct vm_area_struct *vma;
>
> down_write(&mm->mmap_sem);
> - for (vma = mm->mmap; vma; vma = vma->vm_next)
> + for (vma = mm->mmap; vma; vma = vma->vm_next) {
> + vm_write_begin(vma);
> mpol_rebind_policy(vma->vm_policy, new);
> + vm_write_end(vma);
> + }
> up_write(&mm->mmap_sem);
> }
>
> @@ -554,9 +557,11 @@ unsigned long change_prot_numa(struct vm_area_struct *vma,
> {
> int nr_updated;
>
> + vm_write_begin(vma);
> nr_updated = change_protection(vma, addr, end, PAGE_NONE, 0, 1);
> if (nr_updated)
> count_vm_numa_events(NUMA_PTE_UPDATES, nr_updated);
> + vm_write_end(vma);
>
> return nr_updated;
> }
> @@ -657,6 +662,7 @@ static int vma_replace_policy(struct vm_area_struct *vma,
> if (IS_ERR(new))
> return PTR_ERR(new);
>
> + vm_write_begin(vma);
> if (vma->vm_ops && vma->vm_ops->set_policy) {
> err = vma->vm_ops->set_policy(vma, new);
> if (err)
> @@ -664,11 +670,17 @@ static int vma_replace_policy(struct vm_area_struct *vma,
> }
>
> old = vma->vm_policy;
> - vma->vm_policy = new; /* protected by mmap_sem */
> + /*
> + * The speculative page fault handler access this field without
> + * hodling the mmap_sem.
> + */
"The speculative page fault handler accesses this field without holding
vma->vm_mm->mmap_sem"
> + WRITE_ONCE(vma->vm_policy, new);
> + vm_write_end(vma);
> mpol_put(old);
>
> return 0;
> err_out:
> + vm_write_end(vma);
> mpol_put(new);
> return err;
> }
Wait, doesn't vma_dup_policy() also need to protect dst->vm_policy?
diff --git a/mm/mempolicy.c b/mm/mempolicy.c
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -2121,7 +2121,9 @@ int vma_dup_policy(struct vm_area_struct *src, struct vm_area_struct *dst)
if (IS_ERR(pol))
return PTR_ERR(pol);
- dst->vm_policy = pol;
+ vm_write_begin(dst);
+ WRITE_ONCE(dst->vm_policy, pol);
+ vm_write_end(dst);
return 0;
}
^ permalink raw reply
* Re: RFC on writel and writel_relaxed
From: Benjamin Herrenschmidt @ 2018-03-27 21:35 UTC (permalink / raw)
To: Sinan Kaya, Arnd Bergmann, Jason Gunthorpe
Cc: David Laight, Oliver,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT),
linux-rdma@vger.kernel.org, Alexander Duyck, Will Deacon,
Paul E. McKenney, netdev@vger.kernel.org, Alexander Duyck
In-Reply-To: <1e077f6a-90b6-cce9-6f0f-a8c003fec850@codeaurora.org>
On Tue, 2018-03-27 at 10:46 -0400, Sinan Kaya wrote:
> combined buffers.
>
> Alex:
> "Don't bother. I can tell you right now that for x86 you have to have a
> wmb() before the writel().
No, this isn't the semantics of writel. You shouldn't need it unless
something changed and we need to revisit our complete understanding of
*all* MMIO accessor semantics.
At least for UC space, it has always been accepted (and enforced) that
writel would not require any other barrier to order vs. previous stores
to memory.
> Based on the comment in
> (https://www.spinics.net/lists/linux-rdma/msg62666.html):
> Replacing wmb() + writel() with wmb() + writel_relaxed() will work on
> PPC, it will just not give you a benefit today.
>
> I say the patch set stays. This gives benefit on ARM, and has no
> effect on x86 and PowerPC. If you want to look at trying to optimize
> things further on PowerPC and such then go for it in terms of trying
> to implement the writel_relaxed(). Otherwise I say we call the ARM
> goodness a win and don't get ourselves too wrapped up in trying to fix
> this for all architectures."
^ permalink raw reply
* Re: RFC on writel and writel_relaxed
From: Benjamin Herrenschmidt @ 2018-03-27 21:33 UTC (permalink / raw)
To: Alexander Duyck, Will Deacon
Cc: Sinan Kaya, Arnd Bergmann, Jason Gunthorpe, David Laight, Oliver,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT),
linux-rdma@vger.kernel.org, Alexander Duyck, Paul E. McKenney,
netdev@vger.kernel.org, Linus Torvalds
In-Reply-To: <CAKgT0Uem6HT-Z=HBnSVcC4rnDL0yFyXEbr+FN3YYkJ=ExZpUPQ@mail.gmail.com>
On Tue, 2018-03-27 at 11:54 -0700, Alexander Duyck wrote:
> As far as I know the code has been this way for a while, something
> like 2002, when the barrier was already present in e1000. However
> there it was calling out weakly ordered models "such as IA-64". Since
> then pretty much all the hardware based network drivers at this point
> have similar code floating around with wmb() in place to prevent
> issues on weak ordered memory systems.
>
> So in any case we still need to be careful as there are architectures
> that are depending on this even if they might not be x86. :-/
Well, we need to clarify that once and for all, because as I wrote
earlier, it was decreed by Linus more than a decade ago that writel
would be fully ordered by itself vs. previous memory stores (at least
on UC memory).
This is why we added sync's to writel on powerpc and later ARM added
similar barriers to theirs.
This is also why writel_relaxed was added (though much later), since
what writel_relaxed does is to life that specific requirement.
IE. If what you say is true and wmb() is needed on x86, then
writel_relaxed is now completely useless...
Ben.
^ permalink raw reply
* Re: [PATCH v9 07/24] mm: VMA sequence count
From: David Rientjes @ 2018-03-27 21:30 UTC (permalink / raw)
To: Laurent Dufour
Cc: paulmck, peterz, akpm, kirill, ak, mhocko, dave, jack,
Matthew Wilcox, benh, mpe, paulus, Thomas Gleixner, Ingo Molnar,
hpa, Will Deacon, Sergey Senozhatsky, Andrea Arcangeli,
Alexei Starovoitov, kemi.wang, sergey.senozhatsky.work,
Daniel Jordan, linux-kernel, linux-mm, haren, khandual, npiggin,
bsingharora, Tim Chen, linuxppc-dev, x86
In-Reply-To: <1520963994-28477-8-git-send-email-ldufour@linux.vnet.ibm.com>
On Tue, 13 Mar 2018, Laurent Dufour wrote:
> diff --git a/mm/mmap.c b/mm/mmap.c
> index faf85699f1a1..5898255d0aeb 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -558,6 +558,10 @@ void __vma_link_rb(struct mm_struct *mm, struct vm_area_struct *vma,
> else
> mm->highest_vm_end = vm_end_gap(vma);
>
> +#ifdef CONFIG_SPECULATIVE_PAGE_FAULT
> + seqcount_init(&vma->vm_sequence);
> +#endif
> +
> /*
> * vma->vm_prev wasn't known when we followed the rbtree to find the
> * correct insertion point for that vma. As a result, we could not
> @@ -692,6 +696,30 @@ int __vma_adjust(struct vm_area_struct *vma, unsigned long start,
> long adjust_next = 0;
> int remove_next = 0;
>
> + /*
> + * Why using vm_raw_write*() functions here to avoid lockdep's warning ?
> + *
> + * Locked is complaining about a theoretical lock dependency, involving
> + * 3 locks:
> + * mapping->i_mmap_rwsem --> vma->vm_sequence --> fs_reclaim
> + *
> + * Here are the major path leading to this dependency :
> + * 1. __vma_adjust() mmap_sem -> vm_sequence -> i_mmap_rwsem
> + * 2. move_vmap() mmap_sem -> vm_sequence -> fs_reclaim
> + * 3. __alloc_pages_nodemask() fs_reclaim -> i_mmap_rwsem
> + * 4. unmap_mapping_range() i_mmap_rwsem -> vm_sequence
> + *
> + * So there is no way to solve this easily, especially because in
> + * unmap_mapping_range() the i_mmap_rwsem is grab while the impacted
> + * VMAs are not yet known.
> + * However, the way the vm_seq is used is guarantying that we will
> + * never block on it since we just check for its value and never wait
> + * for it to move, see vma_has_changed() and handle_speculative_fault().
> + */
> + vm_raw_write_begin(vma);
> + if (next)
> + vm_raw_write_begin(next);
> +
> if (next && !insert) {
> struct vm_area_struct *exporter = NULL, *importer = NULL;
>
Eek, what about later on:
/*
* Easily overlooked: when mprotect shifts the boundary,
* make sure the expanding vma has anon_vma set if the
* shrinking vma had, to cover any anon pages imported.
*/
if (exporter && exporter->anon_vma && !importer->anon_vma) {
int error;
importer->anon_vma = exporter->anon_vma;
error = anon_vma_clone(importer, exporter);
if (error)
return error;
}
This needs
if (error) {
if (next && next != vma)
vm_raw_write_end(next);
vm_raw_write_end(vma);
return error;
}
^ permalink raw reply
* Re: RFC on writel and writel_relaxed
From: Benjamin Herrenschmidt @ 2018-03-27 21:29 UTC (permalink / raw)
To: Will Deacon, Sinan Kaya
Cc: Arnd Bergmann, Jason Gunthorpe, David Laight, Oliver,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT),
linux-rdma@vger.kernel.org, Paul E. McKenney, Peter Zijlstra,
Ingo Molnar, Jonathan Corbet
In-Reply-To: <20180327143628.GA10642@arm.com>
On Tue, 2018-03-27 at 15:36 +0100, Will Deacon wrote:
> > Can we say the same thing for iowrite32() and iowrite32be(). I also see wmb()
> > in front of these.
>
> I don't think so. My reading of memory-barriers.txt says that writeX might
> expand to outX, and outX is not ordered with respect to other types of
> memory.
Ugh ?
My understanding of HW at least is the exact opposite. outX is *more*
ordered if anything, than any other accessors. IO space is completely
synchronous, non posted and ordered afaik.
Cheers,
Ben.
^ permalink raw reply
* Re: RFC on writel and writel_relaxed
From: Benjamin Herrenschmidt @ 2018-03-27 21:27 UTC (permalink / raw)
To: Jason Gunthorpe, okaya
Cc: Arnd Bergmann, David Laight, Oliver,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT), linux-rdma,
Alexander Duyck, Will Deacon, Paul E. McKenney
In-Reply-To: <20180327141215.GA12318@ziepe.ca>
On Tue, 2018-03-27 at 08:12 -0600, Jason Gunthorpe wrote:
> > I have been converting wmb+writel to wmb+writel_relaxed. (About 30 patches)
> >
> > I will have to just remove the wmb and keep writel, then repost.
>
> Okay, but before you do that, can we get a statement how this works
> for WC?
>
> Some of these writels are to WC memory, do they need the wmb()?!?
This is an issue as we don't have well defined semantics for WC.
At this point, I would suggest staying away from that (ie, not changing
them). We need to look into it.
I know for example that on powerpc I cannot give you any weaker
semantic on WC for writel (I have to put a full sync in there), but I
am trying to see if I can make writel_relaxed both work with the
existing semantics and provide combining. But it's not yet a given (our
weaker IO barrier, eieio, isn't architecturally defined to do anything
on G=0 space, looking with the HW guys at what the HW actually does).
Cheers,
Ben.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox