* [RFC PATCH 08/11] powerpc: Add and use LPPACA_SIZE constant
From: Thiago Jung Bauermann @ 2018-08-24 2:59 UTC (permalink / raw)
To: linuxppc-dev
Cc: iommu, linux-kernel, Alexey Kardashevskiy, Anshuman Khandual,
Benjamin Herrenschmidt, Christoph Hellwig, Michael Ellerman,
Mike Anderson, Paul Mackerras, Ram Pai, Thiago Jung Bauermann
In-Reply-To: <20180824025933.24319-1-bauerman@linux.ibm.com>
Helps document what the hard-coded number means.
Suggested-by: Alexey Kardashevskiy <aik@linux.ibm.com>
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
---
arch/powerpc/kernel/paca.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kernel/paca.c b/arch/powerpc/kernel/paca.c
index 0ee3e6d50f28..1edf8695019d 100644
--- a/arch/powerpc/kernel/paca.c
+++ b/arch/powerpc/kernel/paca.c
@@ -24,6 +24,8 @@
#define boot_cpuid 0
#endif
+#define LPPACA_SIZE 0x400
+
static void *__init alloc_paca_data(unsigned long size, unsigned long align,
unsigned long limit, int cpu)
{
@@ -70,7 +72,7 @@ static inline void init_lppaca(struct lppaca *lppaca)
*lppaca = (struct lppaca) {
.desc = cpu_to_be32(0xd397d781), /* "LpPa" */
- .size = cpu_to_be16(0x400),
+ .size = cpu_to_be16(LPPACA_SIZE),
.fpregs_in_use = 1,
.slb_count = cpu_to_be16(64),
.vmxregs_in_use = 0,
@@ -80,14 +82,13 @@ static inline void init_lppaca(struct lppaca *lppaca)
static struct lppaca * __init new_lppaca(int cpu, unsigned long limit)
{
struct lppaca *lp;
- size_t size = 0x400;
- BUILD_BUG_ON(size < sizeof(struct lppaca));
+ BUILD_BUG_ON(LPPACA_SIZE < sizeof(struct lppaca));
if (early_cpu_has_feature(CPU_FTR_HVMODE))
return NULL;
- lp = alloc_paca_data(size, 0x400, limit, cpu);
+ lp = alloc_paca_data(LPPACA_SIZE, 0x400, limit, cpu);
init_lppaca(lp);
return lp;
^ permalink raw reply related
* [RFC PATCH 09/11] powerpc/svm: Use shared memory for LPPACA structures
From: Thiago Jung Bauermann @ 2018-08-24 2:59 UTC (permalink / raw)
To: linuxppc-dev
Cc: iommu, linux-kernel, Alexey Kardashevskiy, Anshuman Khandual,
Benjamin Herrenschmidt, Christoph Hellwig, Michael Ellerman,
Mike Anderson, Paul Mackerras, Ram Pai, Anshuman Khandual,
Thiago Jung Bauermann
In-Reply-To: <20180824025933.24319-1-bauerman@linux.ibm.com>
From: Anshuman Khandual <khandual@linux.vnet.ibm.com>
LPPACA structures need to be shared with the host. Hence they need to be on
shared memory. Instead of allocating individual chunks of memory for given
structure from memblock, a contiguous chunk of memory is allocated and then
converted into shared memory. Subsequent allocation requests will come from
the contiguous chunk which will be always shared memory for all structures.
While we were able to use a kmem_cache constructor for the Debug Trace Log,
LPPACAs are allocated very early in the boot process (before SLUB is
available) so we need to use a simpler scheme here.
Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
---
arch/powerpc/kernel/paca.c | 34 +++++++++++++++++++++++++++++++++-
1 file changed, 33 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/paca.c b/arch/powerpc/kernel/paca.c
index 1edf8695019d..3e2aca150ad2 100644
--- a/arch/powerpc/kernel/paca.c
+++ b/arch/powerpc/kernel/paca.c
@@ -17,6 +17,7 @@
#include <asm/sections.h>
#include <asm/pgtable.h>
#include <asm/kexec.h>
+#include <asm/svm.h>
#include "setup.h"
@@ -25,6 +26,33 @@
#endif
#define LPPACA_SIZE 0x400
+#define SHARED_LPPACA_SIZE PAGE_ALIGN(LPPACA_SIZE * CONFIG_NR_CPUS)
+
+static phys_addr_t shared_lppaca_pa;
+static unsigned long shared_lppaca_size;
+
+static void *__init alloc_shared_lppaca(unsigned long size, unsigned long align,
+ unsigned long limit, int cpu)
+{
+ unsigned long pa;
+
+ if (!shared_lppaca_pa) {
+ memblock_set_bottom_up(true);
+ shared_lppaca_pa = memblock_alloc_base_nid(SHARED_LPPACA_SIZE,
+ PAGE_SIZE, limit, -1, MEMBLOCK_NONE);
+ if (!shared_lppaca_pa)
+ panic("cannot allocate shared data");
+ memblock_set_bottom_up(false);
+ mem_convert_shared(PHYS_PFN(shared_lppaca_pa),
+ SHARED_LPPACA_SIZE / PAGE_SIZE);
+ }
+
+ pa = shared_lppaca_pa + shared_lppaca_size;
+ shared_lppaca_size += size;
+
+ BUG_ON(shared_lppaca_size >= SHARED_LPPACA_SIZE);
+ return __va(pa);
+}
static void *__init alloc_paca_data(unsigned long size, unsigned long align,
unsigned long limit, int cpu)
@@ -88,7 +116,11 @@ static struct lppaca * __init new_lppaca(int cpu, unsigned long limit)
if (early_cpu_has_feature(CPU_FTR_HVMODE))
return NULL;
- lp = alloc_paca_data(LPPACA_SIZE, 0x400, limit, cpu);
+ if (is_svm_platform())
+ lp = alloc_shared_lppaca(LPPACA_SIZE, 0x400, limit, cpu);
+ else
+ lp = alloc_paca_data(LPPACA_SIZE, 0x400, limit, cpu);
+
init_lppaca(lp);
return lp;
^ permalink raw reply related
* [RFC PATCH 10/11] powerpc/svm: Force the use of bounce buffers
From: Thiago Jung Bauermann @ 2018-08-24 2:59 UTC (permalink / raw)
To: linuxppc-dev
Cc: iommu, linux-kernel, Alexey Kardashevskiy, Anshuman Khandual,
Benjamin Herrenschmidt, Christoph Hellwig, Michael Ellerman,
Mike Anderson, Paul Mackerras, Ram Pai, Anshuman Khandual,
Thiago Jung Bauermann
In-Reply-To: <20180824025933.24319-1-bauerman@linux.ibm.com>
From: Anshuman Khandual <khandual@linux.vnet.ibm.com>
SWIOTLB checks range of incoming CPU addresses to be bounced and see if the
device can access it through it's DMA window without requiring bouncing. In
such cases it just chooses to skip bouncing. But for cases like secure
guests on powerpc platform all addresses need to be bounced into the shared
pool of memory because the host cannot access it otherwise. Hence the need
to do the bouncing is not related to device's DMA window. Hence force the
use of bouncing by setting the swiotlb_force variable on secure guests.
Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
---
arch/powerpc/kernel/svm.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/kernel/svm.c b/arch/powerpc/kernel/svm.c
index 1af5caa955f5..f0576ad65cd0 100644
--- a/arch/powerpc/kernel/svm.c
+++ b/arch/powerpc/kernel/svm.c
@@ -17,6 +17,7 @@ static int __init init_svm(void)
return 0;
ppc_swiotlb_enable = 1;
+ swiotlb_force = SWIOTLB_FORCE;
swiotlb_update_mem_attributes();
return 0;
^ permalink raw reply related
* [RFC PATCH 11/11] powerpc/svm: Increase SWIOTLB buffer size
From: Thiago Jung Bauermann @ 2018-08-24 2:59 UTC (permalink / raw)
To: linuxppc-dev
Cc: iommu, linux-kernel, Alexey Kardashevskiy, Anshuman Khandual,
Benjamin Herrenschmidt, Christoph Hellwig, Michael Ellerman,
Mike Anderson, Paul Mackerras, Ram Pai, Anshuman Khandual,
Thiago Jung Bauermann
In-Reply-To: <20180824025933.24319-1-bauerman@linux.ibm.com>
From: Anshuman Khandual <khandual@linux.vnet.ibm.com>
SWIOTLB buffer default size (64MB) is not enough for large sequential write
operations which eventually leads to kernel crash like here.
virtio-pci 0000:00:05.0: swiotlb buffer is full (sz: 327680 bytes)
virtio-pci 0000:00:05.0: DMA: Out of SW-IOMMU space for 327680 bytes
Kernel panic - not syncing: DMA: Random memory could be DMA read
CPU: 12 PID: 3985 Comm: mkfs.ext4 Not tainted 4.18.0-rc4+ #285
Call Trace:
[c0000007d2a27020] [c000000000cfdffc] dump_stack+0xb0/0xf4 (unreliable)
[c0000007d2a27060] [c000000000112a98] panic+0x140/0x328
[c0000007d2a270f0] [c0000000001b4f88] swiotlb_full+0x108/0x130
[c0000007d2a27180] [c0000000001b5f6c] swiotlb_map_page+0x25c/0x2c0
[c0000007d2a271e0] [c0000000007bfaf8] vring_map_one_sg.isra.0+0x58/0x70
[c0000007d2a27200] [c0000000007c08dc] virtqueue_add_sgs+0x1bc/0x690
[c0000007d2a272f0] [d0000000042a1280] virtio_queue_rq+0x358/0x4a0 [virtio_blk]
[c0000007d2a273d0] [c0000000006b5d68] blk_mq_dispatch_rq_list+0x1f8/0x6d0
..................
Increase the SWIOTLB size to 1GB on Ultravisor based secure guests.
Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
---
arch/powerpc/Kconfig | 5 +++++
kernel/dma/swiotlb.c | 5 +++++
2 files changed, 10 insertions(+)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 1466d1234723..fee7194ce9e4 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -457,6 +457,11 @@ config PPC_SVM
If unsure, say "N".
+config SWIOTLB_DEFAULT_SIZE
+ int "Size of Software I/O TLB buffer (in MiB)"
+ default "1024"
+ depends on PPC_SVM
+
config PPC_TRANSACTIONAL_MEM
bool "Transactional Memory support for POWERPC"
depends on PPC_BOOK3S_64
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 04b68d9dffac..32dc67422d8a 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -146,8 +146,13 @@ void swiotlb_set_max_segment(unsigned int val)
max_segment = rounddown(val, PAGE_SIZE);
}
+#ifdef CONFIG_SWIOTLB_DEFAULT_SIZE
+#define IO_TLB_DEFAULT_SIZE ((unsigned long) CONFIG_SWIOTLB_DEFAULT_SIZE << 20)
+#else
/* default to 64MB */
#define IO_TLB_DEFAULT_SIZE (64UL<<20)
+#endif
+
unsigned long swiotlb_size_or_default(void)
{
unsigned long size;
^ permalink raw reply related
* Re: [PATCH kernel RFC 0/3] powerpc/pseries/iommu: GPU coherent memory pass through
From: Alexey Kardashevskiy @ 2018-08-24 3:04 UTC (permalink / raw)
To: linuxppc-dev
Cc: David Gibson, kvm-ppc, Benjamin Herrenschmidt, Michael Ellerman,
Paul Mackerras, Russell Currey
In-Reply-To: <f5bb1df3-1b15-9c2b-8bb1-7733bb743926@ozlabs.ru>
On 09/08/2018 14:41, Alexey Kardashevskiy wrote:
>
>
> On 25/07/2018 19:50, Alexey Kardashevskiy wrote:
>> I am trying to pass through a 3D controller:
>> [0302]: NVIDIA Corporation GV100GL [Tesla V100 SXM2] [10de:1db1] (rev a1)
>>
>> which has a quite unique feature as coherent memory directly accessible
>> from a POWER9 CPU via an NVLink2 transport.
>>
>> So in addition to passing a PCI device + accompanying NPU devices,
>> we will also be passing the host physical address range as it is done
>> on the bare metal system.
>>
>> The memory on the host is presented as:
>>
>> ===
>> [aik@yc02goos ~]$ lsprop /proc/device-tree/memory@42000000000
>> ibm,chip-id 000000fe (254)
>> device_type "memory"
>> compatible "ibm,coherent-device-memory"
>> reg 00000420 00000000 00000020 00000000
>> linux,usable-memory
>> 00000420 00000000 00000000 00000000
>> phandle 00000726 (1830)
>> name "memory"
>> ibm,associativity
>> 00000004 000000fe 000000fe 000000fe 000000fe
>> ===
>>
>> and the host does not touch it as the second 64bit value of
>> "linux,usable-memory" - the size - is null. Later on the NVIDIA driver
>> trains the NVLink2 and probes this memory and this is how it becomes
>> onlined.
>>
>> In the virtual environment I am planning on doing the same thing,
>> however there is a difference in 64bit DMA handling. The powernv
>> platform uses a PHB3 bypass mode and that just works but
>> the pseries platform uses DDW RTAS API to achieve the same
>> result and the problem with this is that we need a huge DMA
>> window to start from zero (because this GPU supports less than
>> 50bits for DMA address space) and cover not just present memory
>> but also this new coherent memory.
>>
>>
>> This is based on sha1
>> d72e90f3 Linus Torvalds "Linux 4.18-rc6".
>>
>> Please comment. Thanks.
>
>
> Ping?
Ping?
>
>
>>
>>
>>
>> Alexey Kardashevskiy (3):
>> powerpc/pseries/iommu: Allow dynamic window to start from zero
>> powerpc/pseries/iommu: Force default DMA window removal
>> powerpc/pseries/iommu: Use memory@ nodes in max RAM address
>> calculation
>>
>> arch/powerpc/platforms/pseries/iommu.c | 77 ++++++++++++++++++++++++++++++----
>> 1 file changed, 70 insertions(+), 7 deletions(-)
>>
>
--
Alexey
^ permalink raw reply
* Re: DT case sensitivity
From: Michael Ellerman @ 2018-08-24 5:39 UTC (permalink / raw)
To: Rob Herring, Stephen Rothwell, Grant Likely,
Benjamin Herrenschmidt, Kumar Gala, David Gibson, Frank Rowand,
devicetree-spec, devicetree, linuxppc-dev
In-Reply-To: <CAL_Jsq+XacT2O15CQ=F_6dmWf6OqNf8_fKAOrMJNR79a6mORaw@mail.gmail.com>
Rob Herring <robh@kernel.org> writes:
> The default DT string handling in the kernel is node names and
> compatibles are case insensitive and property names are case sensitive
> (Sparc is the the only variation and is opposite). It seems only PPC
> (and perhaps only Power Macs?) needs to support case insensitive
> comparisons. It was probably a mistake to follow PPC for new arches
> and we should have made everything case sensitive from the start. So I
> have a few questions for the DT historians. :)
>
> What PPC systems are case insensitive? Can we limit that to certain systems?
>
> AFAICT, dtc at least (if not anything FDT based) has always been case
> sensitive at least for node and property names. I'm not sure about
> compatible strings?
>
> Anyone see potential issues with switching all platforms except PPC
> and Sparc to case sensitive comparisons?
Looking at some old device trees, on ppc we do definitely have mixed
case compatible strings, and I see a few hundred of those appear in the
kernel source. So I think for us it's too late.
But I don't see why that prevents other arches from switching to case
sensitive, I don't think it really hurts us in any way.
cheers
^ permalink raw reply
* Re: [PATCH v2 3/4] powerpc/mm: fix a warning when a cache is common to PGD and hugepages
From: Michael Ellerman @ 2018-08-24 5:44 UTC (permalink / raw)
To: Segher Boessenkool, Andrew Donnellan
Cc: Russell Currey, Aneesh Kumar K.V, Benjamin Herrenschmidt,
Paul Mackerras, aneesh.kumar, linuxppc-dev, linux-kernel,
Christophe LEROY
In-Reply-To: <20180823144118.GU24439@gate.crashing.org>
Segher Boessenkool <segher@kernel.crashing.org> writes:
> On Thu, Aug 23, 2018 at 11:32:16PM +1000, Andrew Donnellan wrote:
>> On 23/08/18 21:56, Michael Ellerman wrote:
>> >Christophe LEROY <christophe.leroy@c-s.fr> writes:
>> >
>> >>Le 23/08/2018 =C3=A0 12:36, Segher Boessenkool a =C3=A9crit=C2=A0:
>> >>>On Thu, Aug 23, 2018 at 11:40:22AM +0200, Christophe LEROY wrote:
>> >>>>The only small probl=C3=A8me I have is that some version of GCC seem=
s to
>> >>>>complain about big memset() (132k and 256k ones). Is there a way to =
tell
>> >>>>GCC we really want to do it ?
>> >>>
>> >>>I'm not sure what you mean. Complain, is that a warning, is that an=
=20
>> >>>error?
>> >>>What does it say? Do you have some example code to reproduce it? Et=
c.
>> >>
>> >>I saw the warnings in the checks at
>> >>https://patchwork.ozlabs.org/patch/957566/
>> >>Unfortunatly the link is now broken.
>> >
>> >ruscur/ajd any idea what happened to the snowpatch links here?
>>=20
>> I think they've disappeared because our log rotation is too fast - I've=
=20
>> now upped it to 30 days. I guess over time we'll figure out what we need=
=20
>> in this regard, ideally we'd keep logs indefinitely but they're several=
=20
>> megs per build.
>>=20
>> I've kicked off another build for this series and the links in Patchwork=
=20
>> should update to point to the new job when it's done (probably in the=20
>> next couple of hours).
>
> It's back, thanks Andrew!
>
> The warnings are not from GCC at all: the warnings are from sparse.
We really need to split them out so it's less confusing.
cheers
^ permalink raw reply
* Re: [RFC PATCH 00/11] Secure Virtual Machine Enablement
From: Christoph Hellwig @ 2018-08-24 6:00 UTC (permalink / raw)
To: Thiago Jung Bauermann
Cc: linuxppc-dev, iommu, linux-kernel, Alexey Kardashevskiy,
Anshuman Khandual, Benjamin Herrenschmidt, Christoph Hellwig,
Michael Ellerman, Mike Anderson, Paul Mackerras, Ram Pai
In-Reply-To: <20180824025933.24319-1-bauerman@linux.ibm.com>
It seems like you only sent out 6 our of the actual 11 patches according
to the numbering, please resend the full series.
^ permalink raw reply
* Re: [RFC PATCH 10/11] powerpc/svm: Force the use of bounce buffers
From: Christoph Hellwig @ 2018-08-24 6:00 UTC (permalink / raw)
To: Thiago Jung Bauermann
Cc: linuxppc-dev, iommu, linux-kernel, Alexey Kardashevskiy,
Anshuman Khandual, Benjamin Herrenschmidt, Christoph Hellwig,
Michael Ellerman, Mike Anderson, Paul Mackerras, Ram Pai,
Anshuman Khandual
In-Reply-To: <20180824025933.24319-11-bauerman@linux.ibm.com>
On Thu, Aug 23, 2018 at 11:59:32PM -0300, Thiago Jung Bauermann wrote:
> From: Anshuman Khandual <khandual@linux.vnet.ibm.com>
>
> SWIOTLB checks range of incoming CPU addresses to be bounced and see if the
> device can access it through it's DMA window without requiring bouncing. In
> such cases it just chooses to skip bouncing. But for cases like secure
> guests on powerpc platform all addresses need to be bounced into the shared
> pool of memory because the host cannot access it otherwise. Hence the need
> to do the bouncing is not related to device's DMA window. Hence force the
> use of bouncing by setting the swiotlb_force variable on secure guests.
>
> Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> ---
> arch/powerpc/kernel/svm.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/powerpc/kernel/svm.c b/arch/powerpc/kernel/svm.c
> index 1af5caa955f5..f0576ad65cd0 100644
> --- a/arch/powerpc/kernel/svm.c
> +++ b/arch/powerpc/kernel/svm.c
> @@ -17,6 +17,7 @@ static int __init init_svm(void)
> return 0;
>
> ppc_swiotlb_enable = 1;
> + swiotlb_force = SWIOTLB_FORCE;
> swiotlb_update_mem_attributes();
This needs a comment.
^ permalink raw reply
* [PATCH V3 2/6] powerpc/mm/hugetlb/book3s: add _PAGE_PRESENT to hugepd pointer.
From: Aneesh Kumar K.V @ 2018-08-24 6:00 UTC (permalink / raw)
To: npiggin, benh, paulus, mpe; +Cc: linuxppc-dev, Aneesh Kumar K.V
In-Reply-To: <20180824060008.18396-1-aneesh.kumar@linux.ibm.com>
This make hugetlb directory pointer similar to other page able entries. A hugepd
entry is identified by lack of _PAGE_PTE bit set and directory size stored in
HUGEPD_SHIFT_MASK. We update that to also look at _PAGE_PRESENT
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
arch/powerpc/include/asm/book3s/64/hash-4k.h | 2 +-
arch/powerpc/include/asm/book3s/64/hugetlb.h | 3 +++
arch/powerpc/mm/hugetlbpage.c | 2 +-
3 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/64/hash-4k.h b/arch/powerpc/include/asm/book3s/64/hash-4k.h
index 9a3798660cef..15bc16b1dc9c 100644
--- a/arch/powerpc/include/asm/book3s/64/hash-4k.h
+++ b/arch/powerpc/include/asm/book3s/64/hash-4k.h
@@ -66,7 +66,7 @@ static inline int hash__hugepd_ok(hugepd_t hpd)
* if it is not a pte and have hugepd shift mask
* set, then it is a hugepd directory pointer
*/
- if (!(hpdval & _PAGE_PTE) &&
+ if (!(hpdval & _PAGE_PTE) && (hpdval & _PAGE_PRESENT) &&
((hpdval & HUGEPD_SHIFT_MASK) != 0))
return true;
return false;
diff --git a/arch/powerpc/include/asm/book3s/64/hugetlb.h b/arch/powerpc/include/asm/book3s/64/hugetlb.h
index 50888388a359..5b0177733994 100644
--- a/arch/powerpc/include/asm/book3s/64/hugetlb.h
+++ b/arch/powerpc/include/asm/book3s/64/hugetlb.h
@@ -39,4 +39,7 @@ static inline bool gigantic_page_supported(void)
}
#endif
+/* hugepd entry valid bit */
+#define HUGEPD_VAL_BITS (0x8000000000000000UL)
+
#endif
diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
index e87f9ef9115b..c6df73c66c40 100644
--- a/arch/powerpc/mm/hugetlbpage.c
+++ b/arch/powerpc/mm/hugetlbpage.c
@@ -95,7 +95,7 @@ static int __hugepte_alloc(struct mm_struct *mm, hugepd_t *hpdp,
break;
else {
#ifdef CONFIG_PPC_BOOK3S_64
- *hpdp = __hugepd(__pa(new) |
+ *hpdp = __hugepd(__pa(new) | HUGEPD_VAL_BITS |
(shift_to_mmu_psize(pshift) << 2));
#elif defined(CONFIG_PPC_8xx)
*hpdp = __hugepd(__pa(new) | _PMD_USER |
--
2.17.1
^ permalink raw reply related
* [PATCH V3 3/6] powerpc/mm/book3s: Check for pmd_large instead of pmd_trans_huge
From: Aneesh Kumar K.V @ 2018-08-24 6:00 UTC (permalink / raw)
To: npiggin, benh, paulus, mpe; +Cc: linuxppc-dev, Aneesh Kumar K.V
In-Reply-To: <20180824060008.18396-1-aneesh.kumar@linux.ibm.com>
Update few code paths to check for pmd_large.
set_pmd_at:
We want to use this to store swap pte at pmd level. For swap ptes we don't want
to set H_PAGE_THP_HUGE. Hence check for pmd_large in set_pmd_at. This remove
the false WARN_ON when using this with swap pmd entry.
pmd_page:
We don't really use them on pmd migration entries. But they can also work with
migration entries and we don't differentiate at the pte level. Hence update
pmd_page to work with pmd migration entries too
__find_linux_pte:
lockless page table walk need to handle pmd migration entries. pmd_trans_huge
check will return false on them. We don't set thp = 1 for such entries, but
update hpage_shift correctly. Without this we will walk pmd migration entries
as a pte page pointer which is wrong.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
arch/powerpc/mm/hugetlbpage.c | 8 ++++++--
arch/powerpc/mm/pgtable-book3s64.c | 2 +-
arch/powerpc/mm/pgtable_64.c | 2 +-
3 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
index c6df73c66c40..9504641bd4d9 100644
--- a/arch/powerpc/mm/hugetlbpage.c
+++ b/arch/powerpc/mm/hugetlbpage.c
@@ -837,8 +837,12 @@ pte_t *__find_linux_pte(pgd_t *pgdir, unsigned long ea,
ret_pte = (pte_t *) pmdp;
goto out;
}
-
- if (pmd_huge(pmd)) {
+ /*
+ * pmd_large check below will handle the swap pmd pte
+ * we need to do both the check because they are config
+ * dependent.
+ */
+ if (pmd_huge(pmd) || pmd_large(pmd)) {
ret_pte = (pte_t *) pmdp;
goto out;
} else if (is_hugepd(__hugepd(pmd_val(pmd))))
diff --git a/arch/powerpc/mm/pgtable-book3s64.c b/arch/powerpc/mm/pgtable-book3s64.c
index cb73b3f9912c..30bd47c44af3 100644
--- a/arch/powerpc/mm/pgtable-book3s64.c
+++ b/arch/powerpc/mm/pgtable-book3s64.c
@@ -76,7 +76,7 @@ void set_pmd_at(struct mm_struct *mm, unsigned long addr,
WARN_ON((pte_raw(pmd_pte(*pmdp)) & cpu_to_be64(_PAGE_PRESENT)) &&
!pte_protnone(pmd_pte(*pmdp)));
assert_spin_locked(pmd_lockptr(mm, pmdp));
- WARN_ON(!(pmd_trans_huge(pmd) || pmd_devmap(pmd)));
+ WARN_ON(!(pmd_large(pmd) || pmd_devmap(pmd)));
#endif
trace_hugepage_set_pmd(addr, pmd_val(pmd));
return set_pte_at(mm, addr, pmdp_ptep(pmdp), pmd_pte(pmd));
diff --git a/arch/powerpc/mm/pgtable_64.c b/arch/powerpc/mm/pgtable_64.c
index 53e9eeecd5d4..e15e63079ba8 100644
--- a/arch/powerpc/mm/pgtable_64.c
+++ b/arch/powerpc/mm/pgtable_64.c
@@ -306,7 +306,7 @@ struct page *pud_page(pud_t pud)
*/
struct page *pmd_page(pmd_t pmd)
{
- if (pmd_trans_huge(pmd) || pmd_huge(pmd) || pmd_devmap(pmd))
+ if (pmd_large(pmd) || pmd_huge(pmd) || pmd_devmap(pmd))
return pte_page(pmd_pte(pmd));
return virt_to_page(pmd_page_vaddr(pmd));
}
--
2.17.1
^ permalink raw reply related
* [PATCH V3 4/6] arch/powerpc/mm/hash: validate the pte entries before handling the hash fault
From: Aneesh Kumar K.V @ 2018-08-24 6:00 UTC (permalink / raw)
To: npiggin, benh, paulus, mpe; +Cc: linuxppc-dev, Aneesh Kumar K.V
In-Reply-To: <20180824060008.18396-1-aneesh.kumar@linux.ibm.com>
Make sure we are operating on THP and hugetlb entries in the respective hash
fault handling routines.
No functional change in this patch. If we walked the table wrongly before, we
will retry the access.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
arch/powerpc/mm/hugepage-hash64.c | 6 ++++++
arch/powerpc/mm/hugetlbpage-hash64.c | 4 ++++
2 files changed, 10 insertions(+)
diff --git a/arch/powerpc/mm/hugepage-hash64.c b/arch/powerpc/mm/hugepage-hash64.c
index 01f213d2bcb9..dfbc3b32f09b 100644
--- a/arch/powerpc/mm/hugepage-hash64.c
+++ b/arch/powerpc/mm/hugepage-hash64.c
@@ -51,6 +51,12 @@ int __hash_page_thp(unsigned long ea, unsigned long access, unsigned long vsid,
new_pmd |= _PAGE_DIRTY;
} while (!pmd_xchg(pmdp, __pmd(old_pmd), __pmd(new_pmd)));
+ /*
+ * Make sure this is thp or devmap entry
+ */
+ if (!(old_pmd & (H_PAGE_THP_HUGE | _PAGE_DEVMAP)))
+ return 0;
+
rflags = htab_convert_pte_flags(new_pmd);
#if 0
diff --git a/arch/powerpc/mm/hugetlbpage-hash64.c b/arch/powerpc/mm/hugetlbpage-hash64.c
index b320f5097a06..2e6a8f9345d3 100644
--- a/arch/powerpc/mm/hugetlbpage-hash64.c
+++ b/arch/powerpc/mm/hugetlbpage-hash64.c
@@ -62,6 +62,10 @@ int __hash_page_huge(unsigned long ea, unsigned long access, unsigned long vsid,
new_pte |= _PAGE_DIRTY;
} while(!pte_xchg(ptep, __pte(old_pte), __pte(new_pte)));
+ /* Make sure this is a hugetlb entry */
+ if (old_pte & (H_PAGE_THP_HUGE | _PAGE_DEVMAP))
+ return 0;
+
rflags = htab_convert_pte_flags(new_pte);
if (unlikely(mmu_psize == MMU_PAGE_16G))
offset = PTRS_PER_PUD;
--
2.17.1
^ permalink raw reply related
* [PATCH V3 5/6] powerpc/mm/thp: update pmd_trans_huge to check for pmd_present
From: Aneesh Kumar K.V @ 2018-08-24 6:00 UTC (permalink / raw)
To: npiggin, benh, paulus, mpe; +Cc: linuxppc-dev, Aneesh Kumar K.V
In-Reply-To: <20180824060008.18396-1-aneesh.kumar@linux.ibm.com>
We need to make sure pmd_trans_huge returns false for a pmd migration entry.
We mark the migration entry by clearing the _PAGE_PRESENT bit. We keep the
_PAGE_PTE bit set to indicate a leaf page table entry. Hence we need to make
sure we check for pmd_present() so that pmd_trans_huge won't return true on
pmd migration entry.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
.../include/asm/book3s/64/pgtable-64k.h | 3 +++
arch/powerpc/include/asm/book3s/64/pgtable.h | 18 ++++++++++++++++++
2 files changed, 21 insertions(+)
diff --git a/arch/powerpc/include/asm/book3s/64/pgtable-64k.h b/arch/powerpc/include/asm/book3s/64/pgtable-64k.h
index d7ee249d6890..e3d4dd4ae2fa 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable-64k.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable-64k.h
@@ -10,6 +10,9 @@
*
* Defined in such a way that we can optimize away code block at build time
* if CONFIG_HUGETLB_PAGE=n.
+ *
+ * returns true for pmd migration entries, THP, devmap, hugetlb
+ * But compile time dependent on CONFIG_HUGETLB_PAGE
*/
static inline int pmd_huge(pmd_t pmd)
{
diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
index 8feb4a3240d5..e24db2aa260f 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -1136,6 +1136,10 @@ pmd_hugepage_update(struct mm_struct *mm, unsigned long addr, pmd_t *pmdp,
return hash__pmd_hugepage_update(mm, addr, pmdp, clr, set);
}
+/*
+ * returns true for pmd migration entries, THP, devmap, hugetlb
+ * But compile time dependent on THP config
+ */
static inline int pmd_large(pmd_t pmd)
{
return !!(pmd_raw(pmd) & cpu_to_be64(_PAGE_PTE));
@@ -1170,8 +1174,22 @@ static inline void pmdp_set_wrprotect(struct mm_struct *mm, unsigned long addr,
pmd_hugepage_update(mm, addr, pmdp, 0, _PAGE_PRIVILEGED);
}
+/*
+ * Only returns true for a THP. False for pmd migration entry.
+ * We also need to return true when we come across a pte that
+ * in between a thp split. While splitting THP, we mark the pmd
+ * invalid (pmdp_invalidate()) before we set it with pte page
+ * address. A pmd_trans_huge() check against a pmd entry during that time
+ * should return true.
+ * We should not call this on a hugetlb entry. We should check for HugeTLB
+ * entry using vma->vm_flags
+ * The page table walk rule is explained in Documentation/vm/transhuge.rst
+ */
static inline int pmd_trans_huge(pmd_t pmd)
{
+ if (!pmd_present(pmd))
+ return false;
+
if (radix_enabled())
return radix__pmd_trans_huge(pmd);
return hash__pmd_trans_huge(pmd);
--
2.17.1
^ permalink raw reply related
* [PATCH V3 6/6] powerpc/mm:book3s: Enable THP migration support
From: Aneesh Kumar K.V @ 2018-08-24 6:00 UTC (permalink / raw)
To: npiggin, benh, paulus, mpe; +Cc: linuxppc-dev, Aneesh Kumar K.V
In-Reply-To: <20180824060008.18396-1-aneesh.kumar@linux.ibm.com>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
arch/powerpc/include/asm/book3s/64/pgtable.h | 8 ++++++++
arch/powerpc/platforms/Kconfig.cputype | 1 +
2 files changed, 9 insertions(+)
diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
index e24db2aa260f..c68cbbff3429 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -741,6 +741,8 @@ static inline bool pte_user(pte_t pte)
*/
#define __pte_to_swp_entry(pte) ((swp_entry_t) { pte_val((pte)) & ~_PAGE_PTE })
#define __swp_entry_to_pte(x) __pte((x).val | _PAGE_PTE)
+#define __pmd_to_swp_entry(pmd) (__pte_to_swp_entry(pmd_pte(pmd)))
+#define __swp_entry_to_pmd(x) (pte_pmd(__swp_entry_to_pte(x)))
#ifdef CONFIG_MEM_SOFT_DIRTY
#define _PAGE_SWP_SOFT_DIRTY (1UL << (SWP_TYPE_BITS + _PAGE_BIT_SWAP_TYPE))
@@ -1091,6 +1093,12 @@ static inline pte_t *pmdp_ptep(pmd_t *pmd)
#define pmd_soft_dirty(pmd) pte_soft_dirty(pmd_pte(pmd))
#define pmd_mksoft_dirty(pmd) pte_pmd(pte_mksoft_dirty(pmd_pte(pmd)))
#define pmd_clear_soft_dirty(pmd) pte_pmd(pte_clear_soft_dirty(pmd_pte(pmd)))
+
+#ifdef CONFIG_ARCH_ENABLE_THP_MIGRATION
+#define pmd_swp_mksoft_dirty(pmd) pte_pmd(pte_swp_mksoft_dirty(pmd_pte(pmd)))
+#define pmd_swp_soft_dirty(pmd) pte_swp_soft_dirty(pmd_pte(pmd))
+#define pmd_swp_clear_soft_dirty(pmd) pte_pmd(pte_swp_clear_soft_dirty(pmd_pte(pmd)))
+#endif
#endif /* CONFIG_HAVE_ARCH_SOFT_DIRTY */
#ifdef CONFIG_NUMA_BALANCING
diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
index 6c6a7c72cae4..495db17dcbca 100644
--- a/arch/powerpc/platforms/Kconfig.cputype
+++ b/arch/powerpc/platforms/Kconfig.cputype
@@ -72,6 +72,7 @@ config PPC_BOOK3S_64
select PPC_HAVE_PMU_SUPPORT
select SYS_SUPPORTS_HUGETLBFS
select HAVE_ARCH_TRANSPARENT_HUGEPAGE
+ select ARCH_ENABLE_THP_MIGRATION if TRANSPARENT_HUGEPAGE
select ARCH_SUPPORTS_NUMA_BALANCING
select IRQ_WORK
--
2.17.1
^ permalink raw reply related
* Re: [PATCH V3 1/6] powerpc/mm/book3s: Update pmd_present to look at _PAGE_PRESENT bit
From: Aneesh Kumar K.V @ 2018-08-24 6:02 UTC (permalink / raw)
To: npiggin, benh, paulus, mpe; +Cc: linuxppc-dev
In-Reply-To: <20180824060008.18396-1-aneesh.kumar@linux.ibm.com>
On 08/24/2018 11:30 AM, Aneesh Kumar K.V wrote:
> With this patch we use 0x8000000000000000UL (_PAGE_PRESENT) to indicate a valid
> pgd/pud/pmd entry. We also switch the p**_present() to look at this bit.
>
> With pmd_present, we have a special case. We need to make sure we consider a
> pmd marked invalid during THP split as present. Right now we clear the
> _PAGE_PRESENT bit during a pmdp_invalidate. Inorder to consider this special
> case we add a new pte bit _PAGE_INVALID (mapped to _RPAGE_SW0). This bit is
> only used with _PAGE_PRESENT cleared. Hence we are not really losing a pte bit
> for this special case. pmd_present is also updated to look at _PAGE_INVALID.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Changese from V2:
* Rebased to mpe/merge
* Also fixup new WARN_ON with pte_present looking at _PAGE_INVALID now.
-aneesh
^ permalink raw reply
* [PATCH V3 1/6] powerpc/mm/book3s: Update pmd_present to look at _PAGE_PRESENT bit
From: Aneesh Kumar K.V @ 2018-08-24 6:00 UTC (permalink / raw)
To: npiggin, benh, paulus, mpe; +Cc: linuxppc-dev, Aneesh Kumar K.V
With this patch we use 0x8000000000000000UL (_PAGE_PRESENT) to indicate a valid
pgd/pud/pmd entry. We also switch the p**_present() to look at this bit.
With pmd_present, we have a special case. We need to make sure we consider a
pmd marked invalid during THP split as present. Right now we clear the
_PAGE_PRESENT bit during a pmdp_invalidate. Inorder to consider this special
case we add a new pte bit _PAGE_INVALID (mapped to _RPAGE_SW0). This bit is
only used with _PAGE_PRESENT cleared. Hence we are not really losing a pte bit
for this special case. pmd_present is also updated to look at _PAGE_INVALID.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
arch/powerpc/include/asm/book3s/64/hash.h | 5 +++++
arch/powerpc/include/asm/book3s/64/pgtable.h | 14 +++++++++++---
arch/powerpc/mm/hash_utils_64.c | 6 +++---
arch/powerpc/mm/pgtable-book3s64.c | 9 +++++++--
arch/powerpc/mm/pgtable.c | 3 ++-
5 files changed, 28 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/64/hash.h b/arch/powerpc/include/asm/book3s/64/hash.h
index d52a51b2ce7b..fcf8b10a209f 100644
--- a/arch/powerpc/include/asm/book3s/64/hash.h
+++ b/arch/powerpc/include/asm/book3s/64/hash.h
@@ -18,6 +18,11 @@
#include <asm/book3s/64/hash-4k.h>
#endif
+/* Bits to set in a PMD/PUD/PGD entry valid bit*/
+#define HASH_PMD_VAL_BITS (0x8000000000000000UL)
+#define HASH_PUD_VAL_BITS (0x8000000000000000UL)
+#define HASH_PGD_VAL_BITS (0x8000000000000000UL)
+
/*
* Size of EA range mapped by our pagetables.
*/
diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
index 13a688fc8cd0..8feb4a3240d5 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -875,8 +875,16 @@ static inline int pmd_none(pmd_t pmd)
static inline int pmd_present(pmd_t pmd)
{
+ /*
+ * A pmd is considerent present if _PAGE_PRESENT is set.
+ * We also need to consider the pmd present which is marked
+ * invalid during a split. Hence we look for _PAGE_INVALID
+ * if we find _PAGE_PRESENT cleared.
+ */
+ if (pmd_raw(pmd) & cpu_to_be64(_PAGE_PRESENT | _PAGE_INVALID))
+ return true;
- return !pmd_none(pmd);
+ return false;
}
static inline int pmd_bad(pmd_t pmd)
@@ -903,7 +911,7 @@ static inline int pud_none(pud_t pud)
static inline int pud_present(pud_t pud)
{
- return !pud_none(pud);
+ return (pud_raw(pud) & cpu_to_be64(_PAGE_PRESENT));
}
extern struct page *pud_page(pud_t pud);
@@ -950,7 +958,7 @@ static inline int pgd_none(pgd_t pgd)
static inline int pgd_present(pgd_t pgd)
{
- return !pgd_none(pgd);
+ return (pgd_raw(pgd) & cpu_to_be64(_PAGE_PRESENT));
}
static inline pte_t pgd_pte(pgd_t pgd)
diff --git a/arch/powerpc/mm/hash_utils_64.c b/arch/powerpc/mm/hash_utils_64.c
index f23a89d8e4ce..8ff03c7205a0 100644
--- a/arch/powerpc/mm/hash_utils_64.c
+++ b/arch/powerpc/mm/hash_utils_64.c
@@ -1001,9 +1001,9 @@ void __init hash__early_init_mmu(void)
* 4k use hugepd format, so for hash set then to
* zero
*/
- __pmd_val_bits = 0;
- __pud_val_bits = 0;
- __pgd_val_bits = 0;
+ __pmd_val_bits = HASH_PMD_VAL_BITS;
+ __pud_val_bits = HASH_PUD_VAL_BITS;
+ __pgd_val_bits = HASH_PGD_VAL_BITS;
__kernel_virt_start = H_KERN_VIRT_START;
__kernel_virt_size = H_KERN_VIRT_SIZE;
diff --git a/arch/powerpc/mm/pgtable-book3s64.c b/arch/powerpc/mm/pgtable-book3s64.c
index 01d7c0f7c4f0..cb73b3f9912c 100644
--- a/arch/powerpc/mm/pgtable-book3s64.c
+++ b/arch/powerpc/mm/pgtable-book3s64.c
@@ -69,7 +69,12 @@ void set_pmd_at(struct mm_struct *mm, unsigned long addr,
pmd_t *pmdp, pmd_t pmd)
{
#ifdef CONFIG_DEBUG_VM
- WARN_ON(pte_present(pmd_pte(*pmdp)) && !pte_protnone(pmd_pte(*pmdp)));
+ /*
+ * pmd during temporary invalidate will be considered present.
+ * So check for _PAGE_PRESENT explicitly.
+ */
+ WARN_ON((pte_raw(pmd_pte(*pmdp)) & cpu_to_be64(_PAGE_PRESENT)) &&
+ !pte_protnone(pmd_pte(*pmdp)));
assert_spin_locked(pmd_lockptr(mm, pmdp));
WARN_ON(!(pmd_trans_huge(pmd) || pmd_devmap(pmd)));
#endif
@@ -106,7 +111,7 @@ pmd_t pmdp_invalidate(struct vm_area_struct *vma, unsigned long address,
{
unsigned long old_pmd;
- old_pmd = pmd_hugepage_update(vma->vm_mm, address, pmdp, _PAGE_PRESENT, 0);
+ old_pmd = pmd_hugepage_update(vma->vm_mm, address, pmdp, _PAGE_PRESENT, _PAGE_INVALID);
flush_pmd_tlb_range(vma, address, address + HPAGE_PMD_SIZE);
/*
* This ensures that generic code that rely on IRQ disabling
diff --git a/arch/powerpc/mm/pgtable.c b/arch/powerpc/mm/pgtable.c
index d71c7777669c..4e065383fbe3 100644
--- a/arch/powerpc/mm/pgtable.c
+++ b/arch/powerpc/mm/pgtable.c
@@ -192,7 +192,8 @@ void set_pte_at(struct mm_struct *mm, unsigned long addr, pte_t *ptep,
* _PAGE_PRESENT, but we can be sure that it is not in hpte.
* Hence we can use set_pte_at for them.
*/
- VM_WARN_ON(pte_present(*ptep) && !pte_protnone(*ptep));
+ VM_WARN_ON((pte_raw(*ptep) & cpu_to_be64(_PAGE_PRESENT)) &&
+ !pte_protnone(*ptep));
/* Add the pte bit when trying to set a pte */
pte = __pte(pte_val(pte) | _PAGE_PTE);
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v1] mm: relax deferred struct page requirements
From: Jiri Slaby @ 2018-08-24 7:32 UTC (permalink / raw)
To: Pavel Tatashin
Cc: mhocko, Steven Sistare, Daniel Jordan, benh, paulus,
Andrew Morton, kirill.shutemov, Reza Arbab, schwidefsky,
Heiko Carstens, x86, LKML, tglx, linuxppc-dev,
Linux Memory Management List, linux-s390, mgorman
In-Reply-To: <CAGM2reYcwyOcKrO=WhB3Cf0FNL3ZearC=KvxmTNUU6rkWviQOg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1978 bytes --]
On 06/19/2018, 09:56 PM, Pavel Tatashin wrote:
> On Tue, Jun 19, 2018 at 9:50 AM Pavel Tatashin
> <pasha.tatashin@oracle.com> wrote:
>>
>> On Sat, Jun 16, 2018 at 4:04 AM Jiri Slaby <jslaby@suse.cz> wrote:
>>>
>>> On 11/21/2017, 08:24 AM, Michal Hocko wrote:
>>>> On Thu 16-11-17 20:46:01, Pavel Tatashin wrote:
>>>>> There is no need to have ARCH_SUPPORTS_DEFERRED_STRUCT_PAGE_INIT,
>>>>> as all the page initialization code is in common code.
>>>>>
>>>>> Also, there is no need to depend on MEMORY_HOTPLUG, as initialization code
>>>>> does not really use hotplug memory functionality. So, we can remove this
>>>>> requirement as well.
>>>>>
>>>>> This patch allows to use deferred struct page initialization on all
>>>>> platforms with memblock allocator.
>>>>>
>>>>> Tested on x86, arm64, and sparc. Also, verified that code compiles on
>>>>> PPC with CONFIG_MEMORY_HOTPLUG disabled.
>>>>
>>>> There is slight risk that we will encounter corner cases on some
>>>> architectures with weird memory layout/topology
>>>
>>> Which x86_32-pae seems to be. Many bad page state errors are emitted
>>> during boot when this patch is applied:
>>
>> Hi Jiri,
>>
>> Thank you for reporting this bug.
>>
>> Because 32-bit systems are limited in the maximum amount of physical
>> memory, they don't need deferred struct pages. So, we can add depends
>> on 64BIT to DEFERRED_STRUCT_PAGE_INIT in mm/Kconfig.
>>
>> However, before we do this, I want to try reproducing this problem and
>> root cause it, as it might expose a general problem that is not 32-bit
>> specific.
>
> Hi Jiri,
>
> Could you please attach your config and full qemu arguments that you
> used to reproduce this bug.
Hi,
I seem I never replied. Attaching .config and the qemu cmdline:
$ qemu-kvm -m 2000 -hda /dev/null -kernel bzImage
"-m 2000" is important to reproduce.
If I disable CONFIG_DEFERRED_STRUCT_PAGE_INIT (which the patch allowed
to enable), the error goes away, of course.
thanks,
--
js
suse labs
[-- Attachment #2: .config --]
[-- Type: application/x-config, Size: 203953 bytes --]
^ permalink raw reply
* Re: [PATCH v1] mm: relax deferred struct page requirements
From: Jiri Slaby @ 2018-08-24 7:44 UTC (permalink / raw)
To: pavel.tatashin
Cc: mhocko, Steven Sistare, Daniel Jordan, benh, paulus,
Andrew Morton, kirill.shutemov, Reza Arbab, schwidefsky,
Heiko Carstens, x86, LKML, tglx, linuxppc-dev,
Linux Memory Management List, linux-s390, mgorman
In-Reply-To: <83d035f1-40b4-bed8-6113-f4c5a0c4d22f@suse.cz>
pasha.tatashin@oracle.com -> pavel.tatashin@microsoft.com
due to
550 5.1.1 Unknown recipient address.
On 08/24/2018, 09:32 AM, Jiri Slaby wrote:
> On 06/19/2018, 09:56 PM, Pavel Tatashin wrote:
>> On Tue, Jun 19, 2018 at 9:50 AM Pavel Tatashin
>> <pasha.tatashin@oracle.com> wrote:
>>>
>>> On Sat, Jun 16, 2018 at 4:04 AM Jiri Slaby <jslaby@suse.cz> wrote:
>>>>
>>>> On 11/21/2017, 08:24 AM, Michal Hocko wrote:
>>>>> On Thu 16-11-17 20:46:01, Pavel Tatashin wrote:
>>>>>> There is no need to have ARCH_SUPPORTS_DEFERRED_STRUCT_PAGE_INIT,
>>>>>> as all the page initialization code is in common code.
>>>>>>
>>>>>> Also, there is no need to depend on MEMORY_HOTPLUG, as initialization code
>>>>>> does not really use hotplug memory functionality. So, we can remove this
>>>>>> requirement as well.
>>>>>>
>>>>>> This patch allows to use deferred struct page initialization on all
>>>>>> platforms with memblock allocator.
>>>>>>
>>>>>> Tested on x86, arm64, and sparc. Also, verified that code compiles on
>>>>>> PPC with CONFIG_MEMORY_HOTPLUG disabled.
>>>>>
>>>>> There is slight risk that we will encounter corner cases on some
>>>>> architectures with weird memory layout/topology
>>>>
>>>> Which x86_32-pae seems to be. Many bad page state errors are emitted
>>>> during boot when this patch is applied:
>>>
>>> Hi Jiri,
>>>
>>> Thank you for reporting this bug.
>>>
>>> Because 32-bit systems are limited in the maximum amount of physical
>>> memory, they don't need deferred struct pages. So, we can add depends
>>> on 64BIT to DEFERRED_STRUCT_PAGE_INIT in mm/Kconfig.
>>>
>>> However, before we do this, I want to try reproducing this problem and
>>> root cause it, as it might expose a general problem that is not 32-bit
>>> specific.
>>
>> Hi Jiri,
>>
>> Could you please attach your config and full qemu arguments that you
>> used to reproduce this bug.
>
> Hi,
>
> I seem I never replied. Attaching .config and the qemu cmdline:
> $ qemu-kvm -m 2000 -hda /dev/null -kernel bzImage
>
> "-m 2000" is important to reproduce.
>
> If I disable CONFIG_DEFERRED_STRUCT_PAGE_INIT (which the patch allowed
> to enable), the error goes away, of course.
>
> thanks,
>
--
js
suse labs
^ permalink raw reply
* [PATCH 1/3] powerpc/32: Add ioremap_wt()
From: Christophe Leroy @ 2018-08-24 10:04 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Bartlomiej Zolnierkiewicz, Jens Axboe
Cc: linux-kernel, linuxppc-dev, dri-devel, linux-fbdev, linux-block,
linux-m68k
Other arches have ioremap_wt() to map IO areas write-through.
Implement it on PPC as well in order to avoid drivers using
__ioremap(_PAGE_WRITETHRU)
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/include/asm/io.h | 6 ++++++
arch/powerpc/mm/pgtable_32.c | 8 ++++++++
2 files changed, 14 insertions(+)
diff --git a/arch/powerpc/include/asm/io.h b/arch/powerpc/include/asm/io.h
index e0331e754568..3380b5b22450 100644
--- a/arch/powerpc/include/asm/io.h
+++ b/arch/powerpc/include/asm/io.h
@@ -3,6 +3,9 @@
#ifdef __KERNEL__
#define ARCH_HAS_IOREMAP_WC
+#ifdef CONFIG_PPC32
+#define ARCH_HAS_IOREMAP_WT
+#endif
/*
* This program is free software; you can redistribute it and/or
@@ -746,6 +749,8 @@ static inline void iosync(void)
*
* * ioremap_wc enables write combining
*
+ * * ioremap_wt enables write through
+ *
* * iounmap undoes such a mapping and can be hooked
*
* * __ioremap_at (and the pending __iounmap_at) are low level functions to
@@ -767,6 +772,7 @@ extern void __iomem *ioremap(phys_addr_t address, unsigned long size);
extern void __iomem *ioremap_prot(phys_addr_t address, unsigned long size,
unsigned long flags);
extern void __iomem *ioremap_wc(phys_addr_t address, unsigned long size);
+void __iomem *ioremap_wt(phys_addr_t address, unsigned long size);
#define ioremap_nocache(addr, size) ioremap((addr), (size))
#define ioremap_uc(addr, size) ioremap((addr), (size))
#define ioremap_cache(addr, size) \
diff --git a/arch/powerpc/mm/pgtable_32.c b/arch/powerpc/mm/pgtable_32.c
index 2bbc22384f78..b2e73e82f8fc 100644
--- a/arch/powerpc/mm/pgtable_32.c
+++ b/arch/powerpc/mm/pgtable_32.c
@@ -90,6 +90,14 @@ ioremap_wc(phys_addr_t addr, unsigned long size)
EXPORT_SYMBOL(ioremap_wc);
void __iomem *
+ioremap_wt(phys_addr_t addr, unsigned long size)
+{
+ return __ioremap_caller(addr, size, _PAGE_WRITETHRU,
+ __builtin_return_address(0));
+}
+EXPORT_SYMBOL(ioremap_wt);
+
+void __iomem *
ioremap_prot(phys_addr_t addr, unsigned long size, unsigned long flags)
{
pte_t pte = __pte(flags);
--
2.13.3
^ permalink raw reply related
* [PATCH 2/3] drivers/video/fbdev: use ioremap_wt() instead of __ioremap(_PAGE_WRITETHRU)
From: Christophe Leroy @ 2018-08-24 10:04 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Bartlomiej Zolnierkiewicz, Jens Axboe
Cc: linux-kernel, linuxppc-dev, dri-devel, linux-fbdev, linux-block,
linux-m68k
In-Reply-To: <4f905a9d9752768c628623276f5e2fb465c947c8.1535104490.git.christophe.leroy@c-s.fr>
_PAGE_WRITETHRU is a target specific flag. Prefer generic functions.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
drivers/video/fbdev/controlfb.c | 3 +--
drivers/video/fbdev/platinumfb.c | 3 +--
drivers/video/fbdev/valkyriefb.c | 10 ++++++----
3 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/video/fbdev/controlfb.c b/drivers/video/fbdev/controlfb.c
index 8d14b29aafea..058093990218 100644
--- a/drivers/video/fbdev/controlfb.c
+++ b/drivers/video/fbdev/controlfb.c
@@ -715,8 +715,7 @@ static int __init control_of_init(struct device_node *dp)
goto error_out;
}
/* map at most 8MB for the frame buffer */
- p->frame_buffer = __ioremap(p->frame_buffer_phys, 0x800000,
- _PAGE_WRITETHRU);
+ p->frame_buffer = ioremap_wt(p->frame_buffer_phys, 0x800000);
if (!p->control_regs_phys ||
!request_mem_region(p->control_regs_phys, p->control_regs_size,
diff --git a/drivers/video/fbdev/platinumfb.c b/drivers/video/fbdev/platinumfb.c
index 377d3399a3ad..df79f102fbc6 100644
--- a/drivers/video/fbdev/platinumfb.c
+++ b/drivers/video/fbdev/platinumfb.c
@@ -577,8 +577,7 @@ static int platinumfb_probe(struct platform_device* odev)
/* frame buffer - map only 4MB */
pinfo->frame_buffer_phys = pinfo->rsrc_fb.start;
- pinfo->frame_buffer = __ioremap(pinfo->rsrc_fb.start, 0x400000,
- _PAGE_WRITETHRU);
+ pinfo->frame_buffer = ioremap_wt(pinfo->rsrc_fb.start, 0x400000);
pinfo->base_frame_buffer = pinfo->frame_buffer;
/* registers */
diff --git a/drivers/video/fbdev/valkyriefb.c b/drivers/video/fbdev/valkyriefb.c
index 275fb98236d3..0c6c1eede800 100644
--- a/drivers/video/fbdev/valkyriefb.c
+++ b/drivers/video/fbdev/valkyriefb.c
@@ -318,7 +318,7 @@ static void __init valkyrie_choose_mode(struct fb_info_valkyrie *p)
int __init valkyriefb_init(void)
{
struct fb_info_valkyrie *p;
- unsigned long frame_buffer_phys, cmap_regs_phys, flags;
+ unsigned long frame_buffer_phys, cmap_regs_phys;
int err;
char *option = NULL;
@@ -337,7 +337,6 @@ int __init valkyriefb_init(void)
/* Hardcoded addresses... welcome to 68k Macintosh country :-) */
frame_buffer_phys = 0xf9000000;
cmap_regs_phys = 0x50f24000;
- flags = IOMAP_NOCACHE_SER; /* IOMAP_WRITETHROUGH?? */
#else /* ppc (!CONFIG_MAC) */
{
struct device_node *dp;
@@ -354,7 +353,6 @@ int __init valkyriefb_init(void)
frame_buffer_phys = r.start;
cmap_regs_phys = r.start + 0x304000;
- flags = _PAGE_WRITETHRU;
}
#endif /* ppc (!CONFIG_MAC) */
@@ -369,7 +367,11 @@ int __init valkyriefb_init(void)
}
p->total_vram = 0x100000;
p->frame_buffer_phys = frame_buffer_phys;
- p->frame_buffer = __ioremap(frame_buffer_phys, p->total_vram, flags);
+#ifdef CONFIG_MAC
+ p->frame_buffer = ioremap_nocache(frame_buffer_phys, p->total_vram);
+#else
+ p->frame_buffer = ioremap_wt(frame_buffer_phys, p->total_vram);
+#endif
p->cmap_regs_phys = cmap_regs_phys;
p->cmap_regs = ioremap(p->cmap_regs_phys, 0x1000);
p->valkyrie_regs_phys = cmap_regs_phys+0x6000;
--
2.13.3
^ permalink raw reply related
* [PATCH 3/3] drivers/block/z2ram: use ioremap_wt() instead of __ioremap(_PAGE_WRITETHRU)
From: Christophe Leroy @ 2018-08-24 10:04 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
Bartlomiej Zolnierkiewicz, Jens Axboe
Cc: linux-kernel, linuxppc-dev, dri-devel, linux-fbdev, linux-block,
linux-m68k
In-Reply-To: <4f905a9d9752768c628623276f5e2fb465c947c8.1535104490.git.christophe.leroy@c-s.fr>
_PAGE_WRITETHRU is a target specific flag. Prefer generic functions.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
drivers/block/z2ram.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/block/z2ram.c b/drivers/block/z2ram.c
index d0c5bc4e0703..cfbd70520eeb 100644
--- a/drivers/block/z2ram.c
+++ b/drivers/block/z2ram.c
@@ -190,8 +190,7 @@ static int z2_open(struct block_device *bdev, fmode_t mode)
vfree(vmalloc (size));
}
- vaddr = (unsigned long) __ioremap (paddr, size,
- _PAGE_WRITETHRU);
+ vaddr = (unsigned long)ioremap_wt(paddr, size);
#else
vaddr = (unsigned long)z_remap_nocache_nonser(paddr, size);
--
2.13.3
^ permalink raw reply related
* [PATCH] drivers/video/fbdev: use ioremap_wc() instead of __ioremap(_PAGE_NO_CACHE)
From: Christophe Leroy @ 2018-08-24 10:16 UTC (permalink / raw)
To: Bartlomiej Zolnierkiewicz
Cc: dri-devel, linux-fbdev, linux-kernel, linuxppc-dev
_PAGE_NO_CACHE is a target specific flag. Prefer generic functions.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
drivers/video/fbdev/chipsfb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/chipsfb.c b/drivers/video/fbdev/chipsfb.c
index f103665cad43..48fbc22d6867 100644
--- a/drivers/video/fbdev/chipsfb.c
+++ b/drivers/video/fbdev/chipsfb.c
@@ -401,7 +401,7 @@ static int chipsfb_pci_init(struct pci_dev *dp, const struct pci_device_id *ent)
#endif /* CONFIG_PMAC_BACKLIGHT */
#ifdef CONFIG_PPC
- p->screen_base = __ioremap(addr, 0x200000, _PAGE_NO_CACHE);
+ p->screen_base = ioremap_wc(addr, 0x200000);
#else
p->screen_base = ioremap(addr, 0x200000);
#endif
--
2.13.3
^ permalink raw reply related
* Re: [PATCH V3 1/6] powerpc/mm/book3s: Update pmd_present to look at _PAGE_PRESENT bit
From: kbuild test robot @ 2018-08-24 10:29 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: kbuild-all, npiggin, benh, paulus, mpe, Aneesh Kumar K.V,
linuxppc-dev
In-Reply-To: <20180824060008.18396-1-aneesh.kumar@linux.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 8140 bytes --]
Hi Aneesh,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on powerpc/next]
[also build test ERROR on v4.18 next-20180822]
[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/Aneesh-Kumar-K-V/powerpc-mm-book3s-Update-pmd_present-to-look-at-_PAGE_PRESENT-bit/20180824-141837
base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-allnoconfig (attached as .config)
compiler: powerpc-linux-gnu-gcc (Debian 7.2.0-11) 7.2.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=7.2.0 make.cross ARCH=powerpc
All errors (new ones prefixed by >>):
In file included from include/linux/kernel.h:15:0,
from arch/powerpc/mm/pgtable.c:24:
arch/powerpc/mm/pgtable.c: In function 'set_pte_at':
>> arch/powerpc/mm/pgtable.c:195:14: error: implicit declaration of function 'pte_raw'; did you mean 'pte_read'? [-Werror=implicit-function-declaration]
VM_WARN_ON((pte_raw(*ptep) & cpu_to_be64(_PAGE_PRESENT)) &&
^
include/linux/build_bug.h:36:63: note: in definition of macro 'BUILD_BUG_ON_INVALID'
#define BUILD_BUG_ON_INVALID(e) ((void)(sizeof((__force long)(e))))
^
arch/powerpc/mm/pgtable.c:195:2: note: in expansion of macro 'VM_WARN_ON'
VM_WARN_ON((pte_raw(*ptep) & cpu_to_be64(_PAGE_PRESENT)) &&
^~~~~~~~~~
cc1: all warnings being treated as errors
vim +195 arch/powerpc/mm/pgtable.c
> 24 #include <linux/kernel.h>
25 #include <linux/gfp.h>
26 #include <linux/mm.h>
27 #include <linux/percpu.h>
28 #include <linux/hardirq.h>
29 #include <linux/hugetlb.h>
30 #include <asm/pgalloc.h>
31 #include <asm/tlbflush.h>
32 #include <asm/tlb.h>
33
34 static inline int is_exec_fault(void)
35 {
36 return current->thread.regs && TRAP(current->thread.regs) == 0x400;
37 }
38
39 /* We only try to do i/d cache coherency on stuff that looks like
40 * reasonably "normal" PTEs. We currently require a PTE to be present
41 * and we avoid _PAGE_SPECIAL and cache inhibited pte. We also only do that
42 * on userspace PTEs
43 */
44 static inline int pte_looks_normal(pte_t pte)
45 {
46
47 #if defined(CONFIG_PPC_BOOK3S_64)
48 if ((pte_val(pte) & (_PAGE_PRESENT | _PAGE_SPECIAL)) == _PAGE_PRESENT) {
49 if (pte_ci(pte))
50 return 0;
51 if (pte_user(pte))
52 return 1;
53 }
54 return 0;
55 #else
56 return (pte_val(pte) &
57 (_PAGE_PRESENT | _PAGE_SPECIAL | _PAGE_NO_CACHE | _PAGE_USER |
58 _PAGE_PRIVILEGED)) ==
59 (_PAGE_PRESENT | _PAGE_USER);
60 #endif
61 }
62
63 static struct page *maybe_pte_to_page(pte_t pte)
64 {
65 unsigned long pfn = pte_pfn(pte);
66 struct page *page;
67
68 if (unlikely(!pfn_valid(pfn)))
69 return NULL;
70 page = pfn_to_page(pfn);
71 if (PageReserved(page))
72 return NULL;
73 return page;
74 }
75
76 #if defined(CONFIG_PPC_STD_MMU) || _PAGE_EXEC == 0
77
78 /* Server-style MMU handles coherency when hashing if HW exec permission
79 * is supposed per page (currently 64-bit only). If not, then, we always
80 * flush the cache for valid PTEs in set_pte. Embedded CPU without HW exec
81 * support falls into the same category.
82 */
83
84 static pte_t set_pte_filter(pte_t pte)
85 {
86 if (radix_enabled())
87 return pte;
88
89 pte = __pte(pte_val(pte) & ~_PAGE_HPTEFLAGS);
90 if (pte_looks_normal(pte) && !(cpu_has_feature(CPU_FTR_COHERENT_ICACHE) ||
91 cpu_has_feature(CPU_FTR_NOEXECUTE))) {
92 struct page *pg = maybe_pte_to_page(pte);
93 if (!pg)
94 return pte;
95 if (!test_bit(PG_arch_1, &pg->flags)) {
96 flush_dcache_icache_page(pg);
97 set_bit(PG_arch_1, &pg->flags);
98 }
99 }
100 return pte;
101 }
102
103 static pte_t set_access_flags_filter(pte_t pte, struct vm_area_struct *vma,
104 int dirty)
105 {
106 return pte;
107 }
108
109 #else /* defined(CONFIG_PPC_STD_MMU) || _PAGE_EXEC == 0 */
110
111 /* Embedded type MMU with HW exec support. This is a bit more complicated
112 * as we don't have two bits to spare for _PAGE_EXEC and _PAGE_HWEXEC so
113 * instead we "filter out" the exec permission for non clean pages.
114 */
115 static pte_t set_pte_filter(pte_t pte)
116 {
117 struct page *pg;
118
119 /* No exec permission in the first place, move on */
120 if (!(pte_val(pte) & _PAGE_EXEC) || !pte_looks_normal(pte))
121 return pte;
122
123 /* If you set _PAGE_EXEC on weird pages you're on your own */
124 pg = maybe_pte_to_page(pte);
125 if (unlikely(!pg))
126 return pte;
127
128 /* If the page clean, we move on */
129 if (test_bit(PG_arch_1, &pg->flags))
130 return pte;
131
132 /* If it's an exec fault, we flush the cache and make it clean */
133 if (is_exec_fault()) {
134 flush_dcache_icache_page(pg);
135 set_bit(PG_arch_1, &pg->flags);
136 return pte;
137 }
138
139 /* Else, we filter out _PAGE_EXEC */
140 return __pte(pte_val(pte) & ~_PAGE_EXEC);
141 }
142
143 static pte_t set_access_flags_filter(pte_t pte, struct vm_area_struct *vma,
144 int dirty)
145 {
146 struct page *pg;
147
148 /* So here, we only care about exec faults, as we use them
149 * to recover lost _PAGE_EXEC and perform I$/D$ coherency
150 * if necessary. Also if _PAGE_EXEC is already set, same deal,
151 * we just bail out
152 */
153 if (dirty || (pte_val(pte) & _PAGE_EXEC) || !is_exec_fault())
154 return pte;
155
156 #ifdef CONFIG_DEBUG_VM
157 /* So this is an exec fault, _PAGE_EXEC is not set. If it was
158 * an error we would have bailed out earlier in do_page_fault()
159 * but let's make sure of it
160 */
161 if (WARN_ON(!(vma->vm_flags & VM_EXEC)))
162 return pte;
163 #endif /* CONFIG_DEBUG_VM */
164
165 /* If you set _PAGE_EXEC on weird pages you're on your own */
166 pg = maybe_pte_to_page(pte);
167 if (unlikely(!pg))
168 goto bail;
169
170 /* If the page is already clean, we move on */
171 if (test_bit(PG_arch_1, &pg->flags))
172 goto bail;
173
174 /* Clean the page and set PG_arch_1 */
175 flush_dcache_icache_page(pg);
176 set_bit(PG_arch_1, &pg->flags);
177
178 bail:
179 return __pte(pte_val(pte) | _PAGE_EXEC);
180 }
181
182 #endif /* !(defined(CONFIG_PPC_STD_MMU) || _PAGE_EXEC == 0) */
183
184 /*
185 * set_pte stores a linux PTE into the linux page table.
186 */
187 void set_pte_at(struct mm_struct *mm, unsigned long addr, pte_t *ptep,
188 pte_t pte)
189 {
190 /*
191 * When handling numa faults, we already have the pte marked
192 * _PAGE_PRESENT, but we can be sure that it is not in hpte.
193 * Hence we can use set_pte_at for them.
194 */
> 195 VM_WARN_ON((pte_raw(*ptep) & cpu_to_be64(_PAGE_PRESENT)) &&
196 !pte_protnone(*ptep));
197
198 /* Add the pte bit when trying to set a pte */
199 pte = __pte(pte_val(pte) | _PAGE_PTE);
200
201 /* Note: mm->context.id might not yet have been assigned as
202 * this context might not have been activated yet when this
203 * is called.
204 */
205 pte = set_pte_filter(pte);
206
207 /* Perform the setting of the PTE */
208 __set_pte_at(mm, addr, ptep, pte, 0);
209 }
210
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 5702 bytes --]
^ permalink raw reply
* Re: [PATCH V3 1/6] powerpc/mm/book3s: Update pmd_present to look at _PAGE_PRESENT bit
From: kbuild test robot @ 2018-08-24 10:34 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: kbuild-all, npiggin, benh, paulus, mpe, Aneesh Kumar K.V,
linuxppc-dev
In-Reply-To: <20180824060008.18396-1-aneesh.kumar@linux.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 8168 bytes --]
Hi Aneesh,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on powerpc/next]
[also build test WARNING on v4.18 next-20180822]
[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/Aneesh-Kumar-K-V/powerpc-mm-book3s-Update-pmd_present-to-look-at-_PAGE_PRESENT-bit/20180824-141837
base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-obs600_defconfig (attached as .config)
compiler: powerpc-linux-gnu-gcc (Debian 7.2.0-11) 7.2.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=7.2.0 make.cross ARCH=powerpc
All warnings (new ones prefixed by >>):
In file included from include/linux/kernel.h:15:0,
from arch/powerpc/mm/pgtable.c:24:
arch/powerpc/mm/pgtable.c: In function 'set_pte_at':
arch/powerpc/mm/pgtable.c:195:14: error: implicit declaration of function 'pte_raw'; did you mean 'pte_read'? [-Werror=implicit-function-declaration]
VM_WARN_ON((pte_raw(*ptep) & cpu_to_be64(_PAGE_PRESENT)) &&
^
include/linux/build_bug.h:36:63: note: in definition of macro 'BUILD_BUG_ON_INVALID'
#define BUILD_BUG_ON_INVALID(e) ((void)(sizeof((__force long)(e))))
^
>> arch/powerpc/mm/pgtable.c:195:2: note: in expansion of macro 'VM_WARN_ON'
VM_WARN_ON((pte_raw(*ptep) & cpu_to_be64(_PAGE_PRESENT)) &&
^~~~~~~~~~
cc1: all warnings being treated as errors
vim +/VM_WARN_ON +195 arch/powerpc/mm/pgtable.c
> 24 #include <linux/kernel.h>
25 #include <linux/gfp.h>
26 #include <linux/mm.h>
27 #include <linux/percpu.h>
28 #include <linux/hardirq.h>
29 #include <linux/hugetlb.h>
30 #include <asm/pgalloc.h>
31 #include <asm/tlbflush.h>
32 #include <asm/tlb.h>
33
34 static inline int is_exec_fault(void)
35 {
36 return current->thread.regs && TRAP(current->thread.regs) == 0x400;
37 }
38
39 /* We only try to do i/d cache coherency on stuff that looks like
40 * reasonably "normal" PTEs. We currently require a PTE to be present
41 * and we avoid _PAGE_SPECIAL and cache inhibited pte. We also only do that
42 * on userspace PTEs
43 */
44 static inline int pte_looks_normal(pte_t pte)
45 {
46
47 #if defined(CONFIG_PPC_BOOK3S_64)
48 if ((pte_val(pte) & (_PAGE_PRESENT | _PAGE_SPECIAL)) == _PAGE_PRESENT) {
49 if (pte_ci(pte))
50 return 0;
51 if (pte_user(pte))
52 return 1;
53 }
54 return 0;
55 #else
56 return (pte_val(pte) &
57 (_PAGE_PRESENT | _PAGE_SPECIAL | _PAGE_NO_CACHE | _PAGE_USER |
58 _PAGE_PRIVILEGED)) ==
59 (_PAGE_PRESENT | _PAGE_USER);
60 #endif
61 }
62
63 static struct page *maybe_pte_to_page(pte_t pte)
64 {
65 unsigned long pfn = pte_pfn(pte);
66 struct page *page;
67
68 if (unlikely(!pfn_valid(pfn)))
69 return NULL;
70 page = pfn_to_page(pfn);
71 if (PageReserved(page))
72 return NULL;
73 return page;
74 }
75
76 #if defined(CONFIG_PPC_STD_MMU) || _PAGE_EXEC == 0
77
78 /* Server-style MMU handles coherency when hashing if HW exec permission
79 * is supposed per page (currently 64-bit only). If not, then, we always
80 * flush the cache for valid PTEs in set_pte. Embedded CPU without HW exec
81 * support falls into the same category.
82 */
83
84 static pte_t set_pte_filter(pte_t pte)
85 {
86 if (radix_enabled())
87 return pte;
88
89 pte = __pte(pte_val(pte) & ~_PAGE_HPTEFLAGS);
90 if (pte_looks_normal(pte) && !(cpu_has_feature(CPU_FTR_COHERENT_ICACHE) ||
91 cpu_has_feature(CPU_FTR_NOEXECUTE))) {
92 struct page *pg = maybe_pte_to_page(pte);
93 if (!pg)
94 return pte;
95 if (!test_bit(PG_arch_1, &pg->flags)) {
96 flush_dcache_icache_page(pg);
97 set_bit(PG_arch_1, &pg->flags);
98 }
99 }
100 return pte;
101 }
102
103 static pte_t set_access_flags_filter(pte_t pte, struct vm_area_struct *vma,
104 int dirty)
105 {
106 return pte;
107 }
108
109 #else /* defined(CONFIG_PPC_STD_MMU) || _PAGE_EXEC == 0 */
110
111 /* Embedded type MMU with HW exec support. This is a bit more complicated
112 * as we don't have two bits to spare for _PAGE_EXEC and _PAGE_HWEXEC so
113 * instead we "filter out" the exec permission for non clean pages.
114 */
115 static pte_t set_pte_filter(pte_t pte)
116 {
117 struct page *pg;
118
119 /* No exec permission in the first place, move on */
120 if (!(pte_val(pte) & _PAGE_EXEC) || !pte_looks_normal(pte))
121 return pte;
122
123 /* If you set _PAGE_EXEC on weird pages you're on your own */
124 pg = maybe_pte_to_page(pte);
125 if (unlikely(!pg))
126 return pte;
127
128 /* If the page clean, we move on */
129 if (test_bit(PG_arch_1, &pg->flags))
130 return pte;
131
132 /* If it's an exec fault, we flush the cache and make it clean */
133 if (is_exec_fault()) {
134 flush_dcache_icache_page(pg);
135 set_bit(PG_arch_1, &pg->flags);
136 return pte;
137 }
138
139 /* Else, we filter out _PAGE_EXEC */
140 return __pte(pte_val(pte) & ~_PAGE_EXEC);
141 }
142
143 static pte_t set_access_flags_filter(pte_t pte, struct vm_area_struct *vma,
144 int dirty)
145 {
146 struct page *pg;
147
148 /* So here, we only care about exec faults, as we use them
149 * to recover lost _PAGE_EXEC and perform I$/D$ coherency
150 * if necessary. Also if _PAGE_EXEC is already set, same deal,
151 * we just bail out
152 */
153 if (dirty || (pte_val(pte) & _PAGE_EXEC) || !is_exec_fault())
154 return pte;
155
156 #ifdef CONFIG_DEBUG_VM
157 /* So this is an exec fault, _PAGE_EXEC is not set. If it was
158 * an error we would have bailed out earlier in do_page_fault()
159 * but let's make sure of it
160 */
161 if (WARN_ON(!(vma->vm_flags & VM_EXEC)))
162 return pte;
163 #endif /* CONFIG_DEBUG_VM */
164
165 /* If you set _PAGE_EXEC on weird pages you're on your own */
166 pg = maybe_pte_to_page(pte);
167 if (unlikely(!pg))
168 goto bail;
169
170 /* If the page is already clean, we move on */
171 if (test_bit(PG_arch_1, &pg->flags))
172 goto bail;
173
174 /* Clean the page and set PG_arch_1 */
175 flush_dcache_icache_page(pg);
176 set_bit(PG_arch_1, &pg->flags);
177
178 bail:
179 return __pte(pte_val(pte) | _PAGE_EXEC);
180 }
181
182 #endif /* !(defined(CONFIG_PPC_STD_MMU) || _PAGE_EXEC == 0) */
183
184 /*
185 * set_pte stores a linux PTE into the linux page table.
186 */
187 void set_pte_at(struct mm_struct *mm, unsigned long addr, pte_t *ptep,
188 pte_t pte)
189 {
190 /*
191 * When handling numa faults, we already have the pte marked
192 * _PAGE_PRESENT, but we can be sure that it is not in hpte.
193 * Hence we can use set_pte_at for them.
194 */
> 195 VM_WARN_ON((pte_raw(*ptep) & cpu_to_be64(_PAGE_PRESENT)) &&
196 !pte_protnone(*ptep));
197
198 /* Add the pte bit when trying to set a pte */
199 pte = __pte(pte_val(pte) | _PAGE_PTE);
200
201 /* Note: mm->context.id might not yet have been assigned as
202 * this context might not have been activated yet when this
203 * is called.
204 */
205 pte = set_pte_filter(pte);
206
207 /* Perform the setting of the PTE */
208 __set_pte_at(mm, addr, ptep, pte, 0);
209 }
210
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 12365 bytes --]
^ permalink raw reply
* Re: [PATCH v2] powerpc/xive: Avoid unitialized variable
From: Cédric Le Goater @ 2018-08-24 6:58 UTC (permalink / raw)
To: Breno Leitao, linuxppc-dev
In-Reply-To: <1535066799-8493-1-git-send-email-leitao@debian.org>
On 08/24/2018 01:26 AM, Breno Leitao wrote:
> From: Breno Leitao <breno.leitao@gmail.com>
>
> Function xive_native_get_ipi() might uses chip_id without it being
> initialized.
>
> This gives the following error on 'smatch' tool:
>
> error: uninitialized symbol 'chip_id'
>
> The suggestion is using xc->chip_id instead of consulting the OF for chip id,
> which is safe since xive_prepare_cpu() should have initialized ->chip_id by
> the time xive_native_get_ipi() is called.
>
> CC: Cédric Le Goater <clg@kaod.org>
> Signed-off-by: Breno Leitao <leitao@debian.org>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Thanks,
C.
> ---
> arch/powerpc/sysdev/xive/native.c | 11 +----------
> 1 file changed, 1 insertion(+), 10 deletions(-)
>
> diff --git a/arch/powerpc/sysdev/xive/native.c b/arch/powerpc/sysdev/xive/native.c
> index 311185b9960a..bd90fd464a3a 100644
> --- a/arch/powerpc/sysdev/xive/native.c
> +++ b/arch/powerpc/sysdev/xive/native.c
> @@ -238,20 +238,11 @@ static bool xive_native_match(struct device_node *node)
> #ifdef CONFIG_SMP
> static int xive_native_get_ipi(unsigned int cpu, struct xive_cpu *xc)
> {
> - struct device_node *np;
> - unsigned int chip_id;
> s64 irq;
>
> - /* Find the chip ID */
> - np = of_get_cpu_node(cpu, NULL);
> - if (np) {
> - if (of_property_read_u32(np, "ibm,chip-id", &chip_id) < 0)
> - chip_id = 0;
> - }
> -
> /* Allocate an IPI and populate info about it */
> for (;;) {
> - irq = opal_xive_allocate_irq(chip_id);
> + irq = opal_xive_allocate_irq(xc->chip_id);
> if (irq == OPAL_BUSY) {
> msleep(1);
> continue;
>
^ 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