* Re: [RFC PATCH 1/1] powerpc/ftrace: Exclude real mode code from
From: Steven Rostedt @ 2018-03-07 17:45 UTC (permalink / raw)
To: Naveen N. Rao
Cc: Benjamin Herrenschmidt, Michael Ellerman, Paul Mackerras,
Anton Blanchard, Nicholas Piggin, linuxppc-dev
In-Reply-To: <ecbf7e8d6895a7d56b4eb60cbf7461bd463086b3.1520435958.git.naveen.n.rao@linux.vnet.ibm.com>
On Wed, 7 Mar 2018 22:16:19 +0530
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:
> We can't take a trap in most parts of real mode code. Instead of adding
> the 'notrace' annotation to all C functions that can be invoked from
> real mode, detect that we are in real mode on ftrace entry and return
> back.
>
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> ---
> This RFC only handles -mprofile-kernel to demonstrate the approach being
> considered. We will need to handle other ftrace entry if we decide to
> continue down this path.
I do prefer this trade off.
> diff --git a/arch/powerpc/kernel/trace/ftrace_64_mprofile.S b/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
> index 3f3e81852422..ecc0e8e38ead 100644
> --- a/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
> +++ b/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
> @@ -56,6 +56,21 @@ _GLOBAL(ftrace_caller)
>
> /* Load special regs for save below */
> mfmsr r8
> +
> + /* Only proceed if we are not in real mode and can take interrupts */
> + andi. r9, r8, MSR_IR|MSR_DR|MSR_RI
> + cmpdi r9, MSR_IR|MSR_DR|MSR_RI
> + beq 1f
OK, I assume this check and branch is negligible compared to the mfmsr
call?
-- Steve
> + mflr r8
> + mtctr r8
> + REST_GPR(9, r1)
> + REST_GPR(8, r1)
> + addi r1, r1, SWITCH_FRAME_SIZE
> + ld r0, LRSAVE(r1)
> + mtlr r0
> + bctr
> +
> +1:
> mfctr r9
> mfxer r10
> mfcr r11
^ permalink raw reply
* [RFC PATCH 1/1] powerpc/ftrace: Exclude real mode code from
From: Naveen N. Rao @ 2018-03-07 16:46 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Michael Ellerman, Paul Mackerras,
Steven Rostedt
Cc: Anton Blanchard, Nicholas Piggin, linuxppc-dev
In-Reply-To: <cover.1520435958.git.naveen.n.rao@linux.vnet.ibm.com>
We can't take a trap in most parts of real mode code. Instead of adding
the 'notrace' annotation to all C functions that can be invoked from
real mode, detect that we are in real mode on ftrace entry and return
back.
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
This RFC only handles -mprofile-kernel to demonstrate the approach being
considered. We will need to handle other ftrace entry if we decide to
continue down this path.
- Naveen
arch/powerpc/kernel/trace/ftrace_64_mprofile.S | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/arch/powerpc/kernel/trace/ftrace_64_mprofile.S b/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
index 3f3e81852422..ecc0e8e38ead 100644
--- a/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
+++ b/arch/powerpc/kernel/trace/ftrace_64_mprofile.S
@@ -56,6 +56,21 @@ _GLOBAL(ftrace_caller)
/* Load special regs for save below */
mfmsr r8
+
+ /* Only proceed if we are not in real mode and can take interrupts */
+ andi. r9, r8, MSR_IR|MSR_DR|MSR_RI
+ cmpdi r9, MSR_IR|MSR_DR|MSR_RI
+ beq 1f
+ mflr r8
+ mtctr r8
+ REST_GPR(9, r1)
+ REST_GPR(8, r1)
+ addi r1, r1, SWITCH_FRAME_SIZE
+ ld r0, LRSAVE(r1)
+ mtlr r0
+ bctr
+
+1:
mfctr r9
mfxer r10
mfcr r11
--
2.16.1
^ permalink raw reply related
* [RFC PATCH 0/1] Exclude real mode code from ftrace
From: Naveen N. Rao @ 2018-03-07 16:46 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Michael Ellerman, Paul Mackerras,
Steven Rostedt
Cc: Anton Blanchard, Nicholas Piggin, linuxppc-dev
If the function tracer is enabled when starting a guest, we get the
below oops:
------------[ cut here ]------------
Delta way too big! 17582052940437522358 ts=17582052944931114496 write stamp = 4493592138
Oops: Bad interrupt in KVM entry/exit code, sig: 6 [#1]
LE SMP NR_CPUS=2048 NUMA PowerNV
Modules linked in:
CPU: 0 PID: 1380 Comm: qemu-system-ppc Not tainted 4.16.0-rc3-nnr+ #148
NIP: c0000000002635f8 LR: c0000000002635f4 CTR: c0000000001c1384
REGS: c0000000fffd1d80 TRAP: 0700 Not tainted (4.16.0-rc3-nnr+)
MSR: 9000000002823003 <SF,HV,VEC,VSX,FP,ME,RI,LE> CR: 28242222 XER: 20000000
CFAR: c000000000144f94 SOFTE: 3
GPR00: c0000000002635f4 c0000000a26931d0 c0000000013fbe00 0000000000000058
GPR04: 0000000000000001 0000000000000000 0000000000000001 0000000000000000
GPR08: 00000000fe8d0000 c000000001287368 c000000001287368
0000000028242224 GPR12: 0000000000002000 c00000000fac0000
c00000000012cd04 c0000000f0279a00 GPR16: c0000000a26938e0
c000000000de2044 c0000000015c5488 0000000000000000 GPR20:
0000000000000000 0000000000000001 0000000000000000 0000000000000001
GPR24: 0000000000000001 0000000000000000 0000000000000000
0000000000000003 GPR28: 0000000000000000 0000000000000000
00000000000003e8 c0000000a2693260 NIP [c0000000002635f8]
rb_handle_timestamp+0x88/0x90
LR [c0000000002635f4] rb_handle_timestamp+0x84/0x90
Call Trace:
[c0000000a26931d0] [c0000000002635f4] rb_handle_timestamp+0x84/0x90 (unreliable)
[c0000000a2693240] [c000000000266d84] ring_buffer_lock_reserve+0x174/0x5d0
[c0000000a26932b0] [c0000000002728a0] trace_function+0x50/0x190
[c0000000a2693310] [c00000000027f000] function_trace_call+0x140/0x170
[c0000000a2693340] [c000000000064c80] ftrace_call+0x4/0xb8
[c0000000a2693510] [c00000000012720c] kvmppc_hv_entry+0x148/0x164
[c0000000a26935b0] [c000000000126ce0] kvmppc_call_hv_entry+0x28/0x124
[c0000000a2693620] [c00000000011dd84] __kvmppc_vcore_entry+0x13c/0x1b8
[c0000000a26937f0] [c00000000011a8c0] kvmppc_run_core+0xec0/0x1e50
[c0000000a26939b0] [c00000000011c6e4] kvmppc_vcpu_run_hv+0x484/0x1270
[c0000000a2693b30] [c0000000000f8ea8] kvmppc_vcpu_run+0x38/0x50
[c0000000a2693b50] [c0000000000f4a8c] kvm_arch_vcpu_ioctl_run+0x28c/0x380
[c0000000a2693be0] [c0000000000e6978] kvm_vcpu_ioctl+0x4c8/0x780
[c0000000a2693d40] [c0000000003e64e8] do_vfs_ioctl+0xd8/0x900
[c0000000a2693de0] [c0000000003e6d7c] SyS_ioctl+0x6c/0x100
[c0000000a2693e30] [c00000000000bc60] system_call+0x58/0x6c
Instruction dump:
2f890000 409effd4 e8c300b0 e8bf0000 39200001 3ce2ffc9 3c62ffc2 38e78808
38638058 992a7032 4bee1939 60000000 <0fe00000> 4bffffa4 3c4c011a 38428800
---[ end trace 6c43107948f7546d ]---
The KVM entry code updates the timebase register based on the guest's
tb_offset, which upsets ftrace ring buffer time stamps resulting in a
WARN_ONCE() in rb_handle_timestamp(). Furthermore, WARN() inserts a trap
instruction which is now hit while we are in guest MMU context,
resulting in the oops above.
The obvious way to address this is to exclude all KVM C code that can be
run when we are in KVM_GUEST_MODE_HOST_HV from ftrace using the
'notrace' annotation (*). But, there are a few problems doing that:
- the list grows quickly since we need to blacklist not just the top
level function, but every other function which those can call and any
and all functions that those can in turn call, and so on...
- even if we do the above, it is hard to ensure that all functions are
covered and that this continues to be the case due to code refactoring
adding new functions.
The other ways to handle this need a slightly larger hammer:
1. exclude all KVM code from ftrace
2. exclude all real mode code from ftrace
(1) is fairly easy to do, but is still not sufficient since we do call
into various mm/ helpers and they will need to be additionally excluded.
It also ends up excluding a lot of KVM code that can still be traced.
(2) is the approach implemented by the subsequent patch (+) and looks
like a reasonable tradeoff since it additionally excludes all real mode
code, rather than just the KVM code. However, I am not completely sure
how much real mode C code we have, that we would like to be able to
trace. So, it would be good to hear what is preferable.
Please let me know your thoughts.
Thanks,
Naveen
-
(*) Afaics, KVM real mode code is not segregated into a separate file
and is not trivial to do. If this is not true, then this may be an
option to consider.
(+) This RFC only handles -mprofile-kernel, and would need to be updated
to deal with other ftrace entry code.
Naveen N. Rao (1):
powerpc/ftrace: Exclude real mode code from being traced
arch/powerpc/kernel/trace/ftrace_64_mprofile.S | 15 +++++++++++++++
1 file changed, 15 insertions(+)
--
2.16.1
^ permalink raw reply
* [PATCH V2] powerpc/mm/hugetlb: initialize the pagetable cache correctly for hugetlb
From: Aneesh Kumar K.V @ 2018-03-07 14:09 UTC (permalink / raw)
To: benh, paulus, mpe; +Cc: linuxppc-dev, Aneesh Kumar K.V
With 64k page size, we have hugetlb pte entries at the pmd and pud level for
book3s64. We don't need to create a separate page table cache for that. With 4k
we need to make sure hugepd page table cache for 16M is placed at PUD level
and 16G at the PGD level.
Simplify all these by not using HUGEPD_PD_SHIFT which is confusing for book3s64.
Without this patch, with 64k page size we create pagetable caches with shift
value 10 and 7 which are not used at all.
Fixes:419df06eea5bfa81("powerpc: Reduce the PTE_INDEX_SIZE")
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
arch/powerpc/mm/hugetlbpage.c | 15 ++++++++++++---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
index 876da2bc1796..3b509b268030 100644
--- a/arch/powerpc/mm/hugetlbpage.c
+++ b/arch/powerpc/mm/hugetlbpage.c
@@ -122,9 +122,6 @@ static int __hugepte_alloc(struct mm_struct *mm, hugepd_t *hpdp,
#if defined(CONFIG_PPC_FSL_BOOK3E) || defined(CONFIG_PPC_8xx)
#define HUGEPD_PGD_SHIFT PGDIR_SHIFT
#define HUGEPD_PUD_SHIFT PUD_SHIFT
-#else
-#define HUGEPD_PGD_SHIFT PUD_SHIFT
-#define HUGEPD_PUD_SHIFT PMD_SHIFT
#endif
/*
@@ -669,12 +666,24 @@ static int __init hugetlbpage_init(void)
if (add_huge_page_size(1ULL << shift) < 0)
continue;
+
+#ifdef CONFIG_PPC_BOOK3S_64
+ if (shift > PGDIR_SHIFT)
+ BUG();
+ else if (shift > PUD_SHIFT)
+ pdshift = PGDIR_SHIFT;
+ else if (shift > PMD_SHIFT)
+ pdshift = PUD_SHIFT;
+ else
+ pdshift = PMD_SHIFT;
+#else
if (shift < HUGEPD_PUD_SHIFT)
pdshift = PMD_SHIFT;
else if (shift < HUGEPD_PGD_SHIFT)
pdshift = PUD_SHIFT;
else
pdshift = PGDIR_SHIFT;
+#endif
/*
* if we have pdshift and shift value same, we don't
* use pgt cache for hugepd.
--
2.14.3
^ permalink raw reply related
* Re: [PATCH 06/10] powerpc/mm/slice: implement slice_check_range_fits
From: Christophe LEROY @ 2018-03-07 13:38 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: linuxppc-dev, Aneesh Kumar K . V
In-Reply-To: <20180307171621.00ec8814@roar.ozlabs.ibm.com>
Le 07/03/2018 à 08:16, Nicholas Piggin a écrit :
> On Wed, 7 Mar 2018 07:12:23 +0100
> Christophe LEROY <christophe.leroy@c-s.fr> wrote:
>
>> Le 07/03/2018 à 00:12, Nicholas Piggin a écrit :
>>> On Tue, 6 Mar 2018 15:41:00 +0100
>>> Christophe LEROY <christophe.leroy@c-s.fr> wrote:
>>>
>>>> Le 06/03/2018 à 14:25, Nicholas Piggin a écrit :
>
>>>>> @@ -596,10 +601,11 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
>>>>> slice_or_mask(&potential_mask, &good_mask);
>>>>> slice_print_mask(" potential", &potential_mask);
>>>>>
>>>>> - if ((addr != 0 || fixed) &&
>>>>> - slice_check_fit(mm, &mask, &potential_mask)) {
>>>>> - slice_dbg(" fits potential !\n");
>>>>> - goto convert;
>>>>> + if (addr || fixed) {
>>>>> + if (slice_check_range_fits(mm, &potential_mask, addr, len)) {
>>>>> + slice_dbg(" fits potential !\n");
>>>>> + goto convert;
>>>>> + }
>>>>
>>>> Why not keep the original structure and just replacing slice_check_fit()
>>>> by slice_check_range_fits() ?
>>>>
>>>> I believe cleanups should not be mixed with real feature changes. If
>>>> needed, you should have a cleanup patch up front the serie.
>>>
>>> For code that is already changing, I think minor cleanups are okay if
>>> they're very simple. Maybe this is getting to the point of needing
>>> another patch. You've made valid points for a lot of other unnecessary
>>> cleanups though, so I'll fix all of those.
>>
>> Ok, that's not a big point, but I like when patches really modifies
>> only the lines they need to modify.
>
> Fair point, and in the end I agree mostly they should do that. But I
> don't think entirely if you can make the code slightly better as you
> go (again, so long as the change is obvious). I think having extra
> patches for trivial cleanups is not that great either.
>
>> Why do we need a double if ?
>>
>> Why not just the following ? With proper alignment of the second line
>> with the open parenthese, it fits in one line
>>
>> if ((addr != 0 || fixed) &&
>> - slice_check_fit(mm, &mask, &potential_mask)) {
>> + slice_check_range_fits(mm, &potential_mask, addr, len)) {
>> slice_dbg(" fits potential !\n");
>> goto convert;
>
> For this case the main motivation was to make this test match the
> form of the same test (with different mask) above here. Doing the
> same thing with different coding styles annoys me.
Yes good point.
Christophe
>
> I think I kept this one but fixed all your other suggestions in
> the v2 series.
>
> Thanks,
> Nick
>
^ permalink raw reply
* [PATCH 2/2] powerpc/mm/keys: Update documentation in key fault handling
From: Aneesh Kumar K.V @ 2018-03-07 13:36 UTC (permalink / raw)
To: benh, paulus, mpe, linuxram; +Cc: linuxppc-dev, Aneesh Kumar K.V
In-Reply-To: <20180307133645.29290-1-aneesh.kumar@linux.vnet.ibm.com>
No functionality change in this patch. Adds more code comments. We also remove
an unnecessary pkey check after we check for pkey error in this patch.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
arch/powerpc/mm/fault.c | 28 ++++++++++++----------------
arch/powerpc/mm/pkeys.c | 11 ++++-------
2 files changed, 16 insertions(+), 23 deletions(-)
diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index 866446cf2d9a..c01d627e687a 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -297,7 +297,12 @@ static bool access_error(bool is_write, bool is_exec,
if (unlikely(!(vma->vm_flags & (VM_READ | VM_EXEC | VM_WRITE))))
return true;
-
+ /*
+ * We should ideally do the vma pkey access check here. But in the
+ * fault path, handle_mm_fault() also does the same check. To avoid
+ * these multiple checks, we skip it here and handle access error due
+ * to pkeys later.
+ */
return false;
}
@@ -518,25 +523,16 @@ static int __do_page_fault(struct pt_regs *regs, unsigned long address,
#ifdef CONFIG_PPC_MEM_KEYS
/*
- * if the HPTE is not hashed, hardware will not detect
- * a key fault. Lets check if we failed because of a
- * software detected key fault.
+ * we skipped checking for access error due to key earlier.
+ * Check that using handle_mm_fault error return.
*/
if (unlikely(fault & VM_FAULT_SIGSEGV) &&
- !arch_vma_access_permitted(vma, flags & FAULT_FLAG_WRITE,
- is_exec, 0)) {
- /*
- * The PGD-PDT...PMD-PTE tree may not have been fully setup.
- * Hence we cannot walk the tree to locate the PTE, to locate
- * the key. Hence let's use vma_pkey() to get the key; instead
- * of get_mm_addr_key().
- */
+ !arch_vma_access_permitted(vma, is_write, is_exec, 0)) {
+
int pkey = vma_pkey(vma);
- if (likely(pkey)) {
- up_read(&mm->mmap_sem);
- return bad_key_fault_exception(regs, address, pkey);
- }
+ up_read(&mm->mmap_sem);
+ return bad_key_fault_exception(regs, address, pkey);
}
#endif /* CONFIG_PPC_MEM_KEYS */
diff --git a/arch/powerpc/mm/pkeys.c b/arch/powerpc/mm/pkeys.c
index ba71c5481f42..56d33056a559 100644
--- a/arch/powerpc/mm/pkeys.c
+++ b/arch/powerpc/mm/pkeys.c
@@ -119,18 +119,15 @@ int pkey_initialize(void)
#else
os_reserved = 0;
#endif
+ initial_allocation_mask = ~0x0;
+ pkey_amr_uamor_mask = ~0x0ul;
+ pkey_iamr_mask = ~0x0ul;
/*
- * Bits are in LE format. NOTE: 1, 0 are reserved.
+ * key 0, 1 are reserved.
* key 0 is the default key, which allows read/write/execute.
* key 1 is recommended not to be used. PowerISA(3.0) page 1015,
* programming note.
*/
- initial_allocation_mask = ~0x0;
-
- /* register mask is in BE format */
- pkey_amr_uamor_mask = ~0x0ul;
- pkey_iamr_mask = ~0x0ul;
-
for (i = 2; i < (pkeys_total - os_reserved); i++) {
initial_allocation_mask &= ~(0x1 << i);
pkey_amr_uamor_mask &= ~(0x3ul << pkeyshift(i));
--
2.14.3
^ permalink raw reply related
* [PATCH 1/2] powerpc/mm/keys: Move pte bits to correct headers
From: Aneesh Kumar K.V @ 2018-03-07 13:36 UTC (permalink / raw)
To: benh, paulus, mpe, linuxram; +Cc: linuxppc-dev, Aneesh Kumar K.V
Memory keys are supported only with hash translation mode. Instead of #ifdef in
generic code move the key related pte bits to respective headers
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/book3s/64/hash-4k.h | 7 +++++++
arch/powerpc/include/asm/book3s/64/hash-64k.h | 7 +++++++
arch/powerpc/include/asm/book3s/64/pgtable.h | 19 -------------------
3 files changed, 14 insertions(+), 19 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/64/hash-4k.h b/arch/powerpc/include/asm/book3s/64/hash-4k.h
index fc3dc6a93939..4103bfc7c223 100644
--- a/arch/powerpc/include/asm/book3s/64/hash-4k.h
+++ b/arch/powerpc/include/asm/book3s/64/hash-4k.h
@@ -33,6 +33,13 @@
#define H_PAGE_THP_HUGE 0x0
#define H_PAGE_COMBO 0x0
+/* memory key bits, only 8 keys supported */
+#define H_PTE_PKEY_BIT0 0
+#define H_PTE_PKEY_BIT1 0
+#define H_PTE_PKEY_BIT2 _RPAGE_RSV3
+#define H_PTE_PKEY_BIT3 _RPAGE_RSV4
+#define H_PTE_PKEY_BIT4 _RPAGE_RSV5
+
/* 8 bytes per each pte entry */
#define H_PTE_FRAG_SIZE_SHIFT (H_PTE_INDEX_SIZE + 3)
#define H_PTE_FRAG_NR (PAGE_SIZE >> H_PTE_FRAG_SIZE_SHIFT)
diff --git a/arch/powerpc/include/asm/book3s/64/hash-64k.h b/arch/powerpc/include/asm/book3s/64/hash-64k.h
index e53728ff29a0..bb880c97b87d 100644
--- a/arch/powerpc/include/asm/book3s/64/hash-64k.h
+++ b/arch/powerpc/include/asm/book3s/64/hash-64k.h
@@ -16,6 +16,13 @@
#define H_PAGE_BUSY _RPAGE_RPN44 /* software: PTE & hash are busy */
#define H_PAGE_HASHPTE _RPAGE_RPN43 /* PTE has associated HPTE */
+/* memory key bits. */
+#define H_PTE_PKEY_BIT0 _RPAGE_RSV1
+#define H_PTE_PKEY_BIT1 _RPAGE_RSV2
+#define H_PTE_PKEY_BIT2 _RPAGE_RSV3
+#define H_PTE_PKEY_BIT3 _RPAGE_RSV4
+#define H_PTE_PKEY_BIT4 _RPAGE_RSV5
+
/*
* We need to differentiate between explicit huge page and THP huge
* page, since THP huge page also need to track real subpage details
diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
index 973199bd4654..c233915abb68 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -60,25 +60,6 @@
/* Max physical address bit as per radix table */
#define _RPAGE_PA_MAX 57
-#ifdef CONFIG_PPC_MEM_KEYS
-#ifdef CONFIG_PPC_64K_PAGES
-#define H_PTE_PKEY_BIT0 _RPAGE_RSV1
-#define H_PTE_PKEY_BIT1 _RPAGE_RSV2
-#else /* CONFIG_PPC_64K_PAGES */
-#define H_PTE_PKEY_BIT0 0 /* _RPAGE_RSV1 is not available */
-#define H_PTE_PKEY_BIT1 0 /* _RPAGE_RSV2 is not available */
-#endif /* CONFIG_PPC_64K_PAGES */
-#define H_PTE_PKEY_BIT2 _RPAGE_RSV3
-#define H_PTE_PKEY_BIT3 _RPAGE_RSV4
-#define H_PTE_PKEY_BIT4 _RPAGE_RSV5
-#else /* CONFIG_PPC_MEM_KEYS */
-#define H_PTE_PKEY_BIT0 0
-#define H_PTE_PKEY_BIT1 0
-#define H_PTE_PKEY_BIT2 0
-#define H_PTE_PKEY_BIT3 0
-#define H_PTE_PKEY_BIT4 0
-#endif /* CONFIG_PPC_MEM_KEYS */
-
/*
* Max physical address bit we will use for now.
*
--
2.14.3
^ permalink raw reply related
* [RFC PATCH] powerpc/mm/radix: Parse disable_radix commandline correctly.
From: Aneesh Kumar K.V @ 2018-03-07 12:25 UTC (permalink / raw)
To: benh, paulus, mpe; +Cc: linuxppc-dev, Aneesh Kumar K.V
kernel parameter disable_radix takes different options
disable_radix=yes|no|1|0 or just disable_radix. When using the later format
we get below error.
`Malformed early option 'disable_radix'`
We also update the command line parsing in prom_init to handle the new format.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
arch/powerpc/kernel/prom_init.c | 16 +++++++++++++---
arch/powerpc/kernel/prom_init_check.sh | 2 +-
arch/powerpc/mm/init_64.c | 2 +-
3 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index d22c41c26bb3..77735a7655ee 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -171,7 +171,7 @@ static unsigned long __initdata prom_tce_alloc_start;
static unsigned long __initdata prom_tce_alloc_end;
#endif
-static bool __initdata prom_radix_disable;
+static bool __initdata prom_radix_disable = !IS_ENABLED(CONFIG_PPC_RADIX_MMU_DEFAULT);
struct platform_support {
bool hash_mmu;
@@ -641,9 +641,19 @@ static void __init early_cmdline_parse(void)
opt = strstr(prom_cmd_line, "disable_radix");
if (opt) {
- prom_debug("Radix disabled from cmdline\n");
- prom_radix_disable = true;
+ opt += 13;
+ if (*opt && *opt == '=') {
+ bool val;
+
+ if (kstrtobool(++opt, &val))
+ prom_radix_disable = true;
+ else
+ prom_radix_disable = val;
+ } else
+ prom_radix_disable = true;
}
+ if (prom_radix_disable)
+ prom_debug("Radix disabled from cmdline\n");
}
#if defined(CONFIG_PPC_PSERIES) || defined(CONFIG_PPC_POWERNV)
diff --git a/arch/powerpc/kernel/prom_init_check.sh b/arch/powerpc/kernel/prom_init_check.sh
index 12640f7e726b..acb6b9226352 100644
--- a/arch/powerpc/kernel/prom_init_check.sh
+++ b/arch/powerpc/kernel/prom_init_check.sh
@@ -19,7 +19,7 @@
WHITELIST="add_reloc_offset __bss_start __bss_stop copy_and_flush
_end enter_prom memcpy memset reloc_offset __secondary_hold
__secondary_hold_acknowledge __secondary_hold_spinloop __start
-strcmp strcpy strlcpy strlen strncmp strstr logo_linux_clut224
+strcmp strcpy strlcpy strlen strncmp strstr kstrtobool logo_linux_clut224
reloc_got2 kernstart_addr memstart_addr linux_banner _stext
__prom_init_toc_start __prom_init_toc_end btext_setup_display TOC."
diff --git a/arch/powerpc/mm/init_64.c b/arch/powerpc/mm/init_64.c
index 63470b06c502..51ce091914f9 100644
--- a/arch/powerpc/mm/init_64.c
+++ b/arch/powerpc/mm/init_64.c
@@ -366,7 +366,7 @@ static int __init parse_disable_radix(char *p)
{
bool val;
- if (strlen(p) == 0)
+ if (!p)
val = true;
else if (kstrtobool(p, &val))
return -EINVAL;
--
2.14.3
^ permalink raw reply related
* [PATCH] powerpc/mm/hash: Move the slb_addr_limit check within PPC_MM_SLICES
From: Aneesh Kumar K.V @ 2018-03-07 12:15 UTC (permalink / raw)
To: benh, paulus, mpe; +Cc: linuxppc-dev, Aneesh Kumar K.V
Should not have any impact, because we always select PP_MM_SLICES these days.
Nevertheless it is good to indicate that slb_addr_limit is available only
with slice code.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
arch/powerpc/mm/slb_low.S | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/mm/slb_low.S b/arch/powerpc/mm/slb_low.S
index c66cb06e73a1..337ef162851d 100644
--- a/arch/powerpc/mm/slb_low.S
+++ b/arch/powerpc/mm/slb_low.S
@@ -166,6 +166,8 @@ END_MMU_FTR_SECTION_IFCLR(MMU_FTR_1T_SEGMENT)
*/
cmpdi r9, 0
bne- 8f
+
+#ifdef CONFIG_PPC_MM_SLICES
/*
* user space make sure we are within the allowed limit
*/
@@ -183,7 +185,6 @@ END_MMU_FTR_SECTION_IFCLR(MMU_FTR_1T_SEGMENT)
* really do dynamic patching unfortunately as processes might flip
* between 4k and 64k standard page size
*/
-#ifdef CONFIG_PPC_MM_SLICES
/* r10 have esid */
cmpldi r10,16
/* below SLICE_LOW_TOP */
--
2.14.3
^ permalink raw reply related
* [PATCH V4 0/3] Add support for 4PB virtual address space on hash
From: Aneesh Kumar K.V @ 2018-03-07 12:14 UTC (permalink / raw)
To: benh, paulus, mpe; +Cc: linuxppc-dev, Aneesh Kumar K.V
This patch series extended the max virtual address space value from 512TB
to 4PB with 64K page size. We do that by allocating one vsid context for
each 512TB range. More details of that is explained in patch 3.
Changes from V3:
* move extended_id to be a union with mm_context_t id. This reduce some
array index complexity.
* Add addr_limit check when handling slb miss for extended context
Changes from V2:
* Rebased on top of slice_mask series from Nick Piggin
* Fixed segfault when mmap with 512TB hint address
Aneesh Kumar K.V (3):
powerpc/mm: Add support for handling > 512TB address in SLB miss
powerpc/mm/hash64: Increase the VA range
powerpc/mm/hash: Don't memset pgd table if not needed
arch/powerpc/include/asm/book3s/64/hash-4k.h | 6 +
arch/powerpc/include/asm/book3s/64/hash-64k.h | 7 +-
arch/powerpc/include/asm/book3s/64/mmu-hash.h | 6 +-
arch/powerpc/include/asm/book3s/64/mmu.h | 26 ++++-
arch/powerpc/include/asm/book3s/64/pgalloc.h | 13 ++-
arch/powerpc/include/asm/processor.h | 16 ++-
arch/powerpc/kernel/exceptions-64s.S | 12 +-
arch/powerpc/mm/copro_fault.c | 2 +-
arch/powerpc/mm/hash_utils_64.c | 4 +-
arch/powerpc/mm/init_64.c | 6 -
arch/powerpc/mm/mmu_context_book3s64.c | 15 ++-
arch/powerpc/mm/pgtable-hash64.c | 2 +-
arch/powerpc/mm/pgtable_64.c | 5 -
arch/powerpc/mm/slb.c | 154 ++++++++++++++++++++++++++
arch/powerpc/mm/slb_low.S | 6 +-
arch/powerpc/mm/tlb_hash64.c | 2 +-
16 files changed, 252 insertions(+), 30 deletions(-)
--
2.14.3
^ permalink raw reply
* [PATCH V4 3/3] powerpc/mm/hash: Don't memset pgd table if not needed
From: Aneesh Kumar K.V @ 2018-03-07 12:14 UTC (permalink / raw)
To: benh, paulus, mpe; +Cc: linuxppc-dev, Aneesh Kumar K.V
In-Reply-To: <20180307121440.20188-1-aneesh.kumar@linux.vnet.ibm.com>
We need to zero-out pgd table only if we share the slab cache with pud/pmd
level caches. With the support of 4PB, we don't share the slab cache anymore.
Instead of removing the code completely hide it within an #ifdef. We don't need
to do this with any other page table level, because they all allocate table
of double the size and we take of initializing the first half corrrectly during
page table zap.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/book3s/64/pgalloc.h | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/book3s/64/pgalloc.h b/arch/powerpc/include/asm/book3s/64/pgalloc.h
index 4746bc68d446..07f0dbac479f 100644
--- a/arch/powerpc/include/asm/book3s/64/pgalloc.h
+++ b/arch/powerpc/include/asm/book3s/64/pgalloc.h
@@ -80,8 +80,19 @@ static inline pgd_t *pgd_alloc(struct mm_struct *mm)
pgd = kmem_cache_alloc(PGT_CACHE(PGD_INDEX_SIZE),
pgtable_gfp_flags(mm, GFP_KERNEL));
+ /*
+ * With hugetlb, we don't clear the second half of the page table.
+ * If we share the same slab cache with the pmd or pud level table,
+ * we need to make sure we zero out the full table on alloc.
+ * With 4K we don't store slot in the second half. Hence we don't
+ * need to do this for 4k.
+ */
+#if (H_PGD_INDEX_SIZE == H_PUD_CACHE_INDEX) || \
+ (H_PGD_INDEX_SIZE == H_PMD_CACHE_INDEX)
+#if defined(CONFIG_HUGETLB_PAGE) && defined(CONFIG_PPC_64K_PAGES)
memset(pgd, 0, PGD_TABLE_SIZE);
-
+#endif
+#endif
return pgd;
}
--
2.14.3
^ permalink raw reply related
* [PATCH V4 2/3] powerpc/mm/hash64: Increase the VA range
From: Aneesh Kumar K.V @ 2018-03-07 12:14 UTC (permalink / raw)
To: benh, paulus, mpe; +Cc: linuxppc-dev, Aneesh Kumar K.V
In-Reply-To: <20180307121440.20188-1-aneesh.kumar@linux.vnet.ibm.com>
This patch increase the max virtual address value to 4PB. With 4K page size
config we continue to limit ourself to 64TB.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/book3s/64/hash-64k.h | 2 +-
arch/powerpc/include/asm/processor.h | 9 ++++++++-
arch/powerpc/mm/init_64.c | 6 ------
arch/powerpc/mm/pgtable_64.c | 5 -----
4 files changed, 9 insertions(+), 13 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/64/hash-64k.h b/arch/powerpc/include/asm/book3s/64/hash-64k.h
index 0ee0fc1ad675..02098d7fe177 100644
--- a/arch/powerpc/include/asm/book3s/64/hash-64k.h
+++ b/arch/powerpc/include/asm/book3s/64/hash-64k.h
@@ -4,7 +4,7 @@
#define H_PTE_INDEX_SIZE 8
#define H_PMD_INDEX_SIZE 10
-#define H_PUD_INDEX_SIZE 7
+#define H_PUD_INDEX_SIZE 10
#define H_PGD_INDEX_SIZE 8
/*
* No of address bits below which we use the default context
diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
index 70d65b482504..a621a068880a 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -109,6 +109,13 @@ void release_thread(struct task_struct *);
#define TASK_SIZE_64TB (0x0000400000000000UL)
#define TASK_SIZE_128TB (0x0000800000000000UL)
#define TASK_SIZE_512TB (0x0002000000000000UL)
+#define TASK_SIZE_1PB (0x0004000000000000UL)
+#define TASK_SIZE_2PB (0x0008000000000000UL)
+/*
+ * With 52 bits in the address we can support
+ * upto 4PB of range.
+ */
+#define TASK_SIZE_4PB (0x0010000000000000UL)
/*
* For now 512TB is only supported with book3s and 64K linux page size.
@@ -117,7 +124,7 @@ void release_thread(struct task_struct *);
/*
* Max value currently used:
*/
-#define TASK_SIZE_USER64 TASK_SIZE_512TB
+#define TASK_SIZE_USER64 TASK_SIZE_4PB
#define DEFAULT_MAP_WINDOW_USER64 TASK_SIZE_128TB
#define TASK_CONTEXT_SIZE TASK_SIZE_512TB
#else
diff --git a/arch/powerpc/mm/init_64.c b/arch/powerpc/mm/init_64.c
index fdb424a29f03..63470b06c502 100644
--- a/arch/powerpc/mm/init_64.c
+++ b/arch/powerpc/mm/init_64.c
@@ -68,12 +68,6 @@
#include "mmu_decl.h"
-#ifdef CONFIG_PPC_BOOK3S_64
-#if H_PGTABLE_RANGE > USER_VSID_RANGE
-#warning Limited user VSID range means pagetable space is wasted
-#endif
-#endif /* CONFIG_PPC_BOOK3S_64 */
-
phys_addr_t memstart_addr = ~0;
EXPORT_SYMBOL_GPL(memstart_addr);
phys_addr_t kernstart_addr;
diff --git a/arch/powerpc/mm/pgtable_64.c b/arch/powerpc/mm/pgtable_64.c
index 28c980eb4422..16636bdf3331 100644
--- a/arch/powerpc/mm/pgtable_64.c
+++ b/arch/powerpc/mm/pgtable_64.c
@@ -57,11 +57,6 @@
#include "mmu_decl.h"
-#ifdef CONFIG_PPC_BOOK3S_64
-#if TASK_SIZE_USER64 > (1UL << (ESID_BITS + SID_SHIFT))
-#error TASK_SIZE_USER64 exceeds user VSID range
-#endif
-#endif
#ifdef CONFIG_PPC_BOOK3S_64
/*
--
2.14.3
^ permalink raw reply related
* [PATCH V4 1/3] powerpc/mm: Add support for handling > 512TB address in SLB miss
From: Aneesh Kumar K.V @ 2018-03-07 12:14 UTC (permalink / raw)
To: benh, paulus, mpe; +Cc: linuxppc-dev, Aneesh Kumar K.V
In-Reply-To: <20180307121440.20188-1-aneesh.kumar@linux.vnet.ibm.com>
For address above 512TB we allocate additional mmu context. To make it all
easy address above 512TB is handled with IR/DR=1 and with stack frame setup.
We do the additional context allocation in SLB miss handler. If the context is
not allocated, we enable interrupts and allocate the context and retry the
access which will again result in a SLB miss.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/book3s/64/hash-4k.h | 6 +
arch/powerpc/include/asm/book3s/64/hash-64k.h | 5 +
arch/powerpc/include/asm/book3s/64/mmu-hash.h | 6 +-
arch/powerpc/include/asm/book3s/64/mmu.h | 26 ++++-
arch/powerpc/include/asm/processor.h | 7 ++
arch/powerpc/kernel/exceptions-64s.S | 12 +-
arch/powerpc/mm/copro_fault.c | 2 +-
arch/powerpc/mm/hash_utils_64.c | 4 +-
arch/powerpc/mm/mmu_context_book3s64.c | 15 ++-
arch/powerpc/mm/pgtable-hash64.c | 2 +-
arch/powerpc/mm/slb.c | 154 ++++++++++++++++++++++++++
arch/powerpc/mm/slb_low.S | 6 +-
arch/powerpc/mm/tlb_hash64.c | 2 +-
13 files changed, 231 insertions(+), 16 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/64/hash-4k.h b/arch/powerpc/include/asm/book3s/64/hash-4k.h
index 67c5475311ee..af2ba9875f18 100644
--- a/arch/powerpc/include/asm/book3s/64/hash-4k.h
+++ b/arch/powerpc/include/asm/book3s/64/hash-4k.h
@@ -11,6 +11,12 @@
#define H_PUD_INDEX_SIZE 9
#define H_PGD_INDEX_SIZE 9
+/*
+ * No of address bits below which we use the default context
+ * for slb allocation. For 4k this is 64TB.
+ */
+#define H_BITS_FIRST_CONTEXT 46
+
#ifndef __ASSEMBLY__
#define H_PTE_TABLE_SIZE (sizeof(pte_t) << H_PTE_INDEX_SIZE)
#define H_PMD_TABLE_SIZE (sizeof(pmd_t) << H_PMD_INDEX_SIZE)
diff --git a/arch/powerpc/include/asm/book3s/64/hash-64k.h b/arch/powerpc/include/asm/book3s/64/hash-64k.h
index 3bcf269f8f55..0ee0fc1ad675 100644
--- a/arch/powerpc/include/asm/book3s/64/hash-64k.h
+++ b/arch/powerpc/include/asm/book3s/64/hash-64k.h
@@ -6,6 +6,11 @@
#define H_PMD_INDEX_SIZE 10
#define H_PUD_INDEX_SIZE 7
#define H_PGD_INDEX_SIZE 8
+/*
+ * No of address bits below which we use the default context
+ * for slb allocation. For 64k this is 512TB.
+ */
+#define H_BITS_FIRST_CONTEXT 49
/*
* 64k aligned address free up few of the lower bits of RPN for us
diff --git a/arch/powerpc/include/asm/book3s/64/mmu-hash.h b/arch/powerpc/include/asm/book3s/64/mmu-hash.h
index 50ed64fba4ae..8ee83f6e9c84 100644
--- a/arch/powerpc/include/asm/book3s/64/mmu-hash.h
+++ b/arch/powerpc/include/asm/book3s/64/mmu-hash.h
@@ -691,8 +691,8 @@ static inline int user_segment_size(unsigned long addr)
return MMU_SEGSIZE_256M;
}
-static inline unsigned long get_vsid(unsigned long context, unsigned long ea,
- int ssize)
+static inline unsigned long __get_vsid(unsigned long context, unsigned long ea,
+ int ssize)
{
unsigned long va_bits = VA_BITS;
unsigned long vsid_bits;
@@ -744,7 +744,7 @@ static inline unsigned long get_kernel_vsid(unsigned long ea, int ssize)
*/
context = (ea >> 60) - KERNEL_REGION_CONTEXT_OFFSET;
- return get_vsid(context, ea, ssize);
+ return __get_vsid(context, ea, ssize);
}
unsigned htab_shift_for_mem_size(unsigned long mem_size);
diff --git a/arch/powerpc/include/asm/book3s/64/mmu.h b/arch/powerpc/include/asm/book3s/64/mmu.h
index 777778579305..a70adbb7ec56 100644
--- a/arch/powerpc/include/asm/book3s/64/mmu.h
+++ b/arch/powerpc/include/asm/book3s/64/mmu.h
@@ -91,7 +91,15 @@ struct slice_mask {
};
typedef struct {
- mm_context_id_t id;
+ union {
+ /*
+ * One context for each 512TB.
+ * First 512TB context is saved in id and is also used
+ * as PIDR.
+ */
+ mm_context_id_t id;
+ mm_context_id_t extended_id[TASK_SIZE_USER64/TASK_CONTEXT_SIZE];
+ };
u16 user_psize; /* page size index */
/* Number of bits in the mm_cpumask */
@@ -193,5 +201,21 @@ extern void radix_init_pseries(void);
static inline void radix_init_pseries(void) { };
#endif
+static inline int get_esid_context(mm_context_t *ctx, unsigned long ea)
+{
+ int index = ea >> H_BITS_FIRST_CONTEXT;
+
+ return ctx->extended_id[index];
+}
+
+static inline unsigned long get_user_vsid(mm_context_t *ctx,
+ unsigned long ea, int ssize)
+{
+ unsigned long context = get_esid_context(ctx, ea);
+
+ return __get_vsid(context, ea, ssize);
+}
+
+
#endif /* __ASSEMBLY__ */
#endif /* _ASM_POWERPC_BOOK3S_64_MMU_H_ */
diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
index 01299cdc9806..70d65b482504 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -119,9 +119,16 @@ void release_thread(struct task_struct *);
*/
#define TASK_SIZE_USER64 TASK_SIZE_512TB
#define DEFAULT_MAP_WINDOW_USER64 TASK_SIZE_128TB
+#define TASK_CONTEXT_SIZE TASK_SIZE_512TB
#else
#define TASK_SIZE_USER64 TASK_SIZE_64TB
#define DEFAULT_MAP_WINDOW_USER64 TASK_SIZE_64TB
+/*
+ * We don't need allocate extended context id for 4K
+ * page size. We limit max address on this config to
+ * 64TB.
+ */
+#define TASK_CONTEXT_SIZE TASK_SIZE_64TB
#endif
/*
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index 3ac87e53b3da..166b8c0f1830 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -620,8 +620,12 @@ END_MMU_FTR_SECTION_IFCLR(MMU_FTR_TYPE_RADIX)
ld r10,PACA_EXSLB+EX_LR(r13)
lwz r9,PACA_EXSLB+EX_CCR(r13) /* get saved CR */
mtlr r10
+ /*
+ * Large address, check whether we have to allocate new
+ * contexts.
+ */
+ beq- 8f
- beq- 8f /* if bad address, make full stack frame */
bne- cr5,2f /* if unrecoverable exception, oops */
@@ -685,7 +689,7 @@ END_MMU_FTR_SECTION_IFCLR(MMU_FTR_TYPE_RADIX)
mr r3,r12
mfspr r11,SPRN_SRR0
mfspr r12,SPRN_SRR1
- LOAD_HANDLER(r10,bad_addr_slb)
+ LOAD_HANDLER(r10, multi_context_slb)
mtspr SPRN_SRR0,r10
ld r10,PACAKMSR(r13)
mtspr SPRN_SRR1,r10
@@ -700,7 +704,7 @@ EXC_COMMON_BEGIN(unrecov_slb)
bl unrecoverable_exception
b 1b
-EXC_COMMON_BEGIN(bad_addr_slb)
+EXC_COMMON_BEGIN(multi_context_slb)
EXCEPTION_PROLOG_COMMON(0x380, PACA_EXSLB)
RECONCILE_IRQ_STATE(r10, r11)
ld r3, PACA_EXSLB+EX_DAR(r13)
@@ -710,7 +714,7 @@ EXC_COMMON_BEGIN(bad_addr_slb)
std r10, _TRAP(r1)
2: bl save_nvgprs
addi r3, r1, STACK_FRAME_OVERHEAD
- bl slb_miss_bad_addr
+ bl handle_multi_context_slb_miss
b ret_from_except
EXC_REAL_BEGIN(hardware_interrupt, 0x500, 0x100)
diff --git a/arch/powerpc/mm/copro_fault.c b/arch/powerpc/mm/copro_fault.c
index 697b70ad1195..7d0945bd3a61 100644
--- a/arch/powerpc/mm/copro_fault.c
+++ b/arch/powerpc/mm/copro_fault.c
@@ -112,7 +112,7 @@ int copro_calculate_slb(struct mm_struct *mm, u64 ea, struct copro_slb *slb)
return 1;
psize = get_slice_psize(mm, ea);
ssize = user_segment_size(ea);
- vsid = get_vsid(mm->context.id, ea, ssize);
+ vsid = get_user_vsid(&mm->context, ea, ssize);
vsidkey = SLB_VSID_USER;
break;
case VMALLOC_REGION_ID:
diff --git a/arch/powerpc/mm/hash_utils_64.c b/arch/powerpc/mm/hash_utils_64.c
index b578148d89e6..f62325d4f5f5 100644
--- a/arch/powerpc/mm/hash_utils_64.c
+++ b/arch/powerpc/mm/hash_utils_64.c
@@ -1261,7 +1261,7 @@ int hash_page_mm(struct mm_struct *mm, unsigned long ea,
}
psize = get_slice_psize(mm, ea);
ssize = user_segment_size(ea);
- vsid = get_vsid(mm->context.id, ea, ssize);
+ vsid = get_user_vsid(&mm->context, ea, ssize);
break;
case VMALLOC_REGION_ID:
vsid = get_kernel_vsid(ea, mmu_kernel_ssize);
@@ -1526,7 +1526,7 @@ void hash_preload(struct mm_struct *mm, unsigned long ea,
/* Get VSID */
ssize = user_segment_size(ea);
- vsid = get_vsid(mm->context.id, ea, ssize);
+ vsid = get_user_vsid(&mm->context, ea, ssize);
if (!vsid)
return;
/*
diff --git a/arch/powerpc/mm/mmu_context_book3s64.c b/arch/powerpc/mm/mmu_context_book3s64.c
index 80acad52b006..ccc88fa7c35c 100644
--- a/arch/powerpc/mm/mmu_context_book3s64.c
+++ b/arch/powerpc/mm/mmu_context_book3s64.c
@@ -178,6 +178,19 @@ void __destroy_context(int context_id)
}
EXPORT_SYMBOL_GPL(__destroy_context);
+static void destroy_contexts(mm_context_t *ctx)
+{
+ int index, context_id;
+
+ spin_lock(&mmu_context_lock);
+ for (index = 0; index < (TASK_SIZE_USER64/TASK_CONTEXT_SIZE); index++) {
+ context_id = ctx->extended_id[index];
+ if (context_id)
+ ida_remove(&mmu_context_ida, context_id);
+ }
+ spin_unlock(&mmu_context_lock);
+}
+
#ifdef CONFIG_PPC_64K_PAGES
static void destroy_pagetable_page(struct mm_struct *mm)
{
@@ -216,7 +229,7 @@ void destroy_context(struct mm_struct *mm)
else
subpage_prot_free(mm);
destroy_pagetable_page(mm);
- __destroy_context(mm->context.id);
+ destroy_contexts(&mm->context);
mm->context.id = MMU_NO_CONTEXT;
}
diff --git a/arch/powerpc/mm/pgtable-hash64.c b/arch/powerpc/mm/pgtable-hash64.c
index 469808e77e58..a87b18cf6749 100644
--- a/arch/powerpc/mm/pgtable-hash64.c
+++ b/arch/powerpc/mm/pgtable-hash64.c
@@ -320,7 +320,7 @@ void hpte_do_hugepage_flush(struct mm_struct *mm, unsigned long addr,
if (!is_kernel_addr(addr)) {
ssize = user_segment_size(addr);
- vsid = get_vsid(mm->context.id, addr, ssize);
+ vsid = get_user_vsid(&mm->context, addr, ssize);
WARN_ON(vsid == 0);
} else {
vsid = get_kernel_vsid(addr, mmu_kernel_ssize);
diff --git a/arch/powerpc/mm/slb.c b/arch/powerpc/mm/slb.c
index 13cfe413b40d..a93887ee9b22 100644
--- a/arch/powerpc/mm/slb.c
+++ b/arch/powerpc/mm/slb.c
@@ -23,6 +23,7 @@
#include <asm/smp.h>
#include <linux/compiler.h>
#include <linux/mm_types.h>
+#include <linux/context_tracking.h>
#include <asm/udbg.h>
#include <asm/code-patching.h>
@@ -340,3 +341,156 @@ void slb_initialize(void)
asm volatile("isync":::"memory");
}
+
+/*
+ * Only handle insert of 1TB slb entries.
+ */
+static void insert_slb_entry(unsigned long vsid, unsigned long ea,
+ int bpsize, int ssize)
+{
+ int slb_cache_index;
+ unsigned long flags;
+ enum slb_index index;
+ unsigned long vsid_data, esid_data;
+
+ /*
+ * We are irq disabled, hence should be safe
+ * to access PACA.
+ */
+ index = get_paca()->stab_rr;
+ /*
+ * simple round roubin replacement of slb.
+ */
+ if (index < mmu_slb_size)
+ index++;
+ else
+ index = SLB_NUM_BOLTED;
+ get_paca()->stab_rr = index;
+
+ flags = SLB_VSID_USER | mmu_psize_defs[bpsize].sllp;
+ vsid_data = (vsid << SLB_VSID_SHIFT_1T) | flags |
+ ((unsigned long) ssize << SLB_VSID_SSIZE_SHIFT);
+ esid_data = mk_esid_data(ea, mmu_highuser_ssize, index);
+
+ asm volatile("slbmte %0, %1" : : "r" (vsid_data), "r" (esid_data)
+ : "memory");
+ /*
+ * Now update slb cache entries
+ */
+ slb_cache_index = get_paca()->slb_cache_ptr;
+ if (slb_cache_index < SLB_CACHE_ENTRIES) {
+ /*
+ * We have space in slb cache for optimized switch_slb().
+ * Top 36 bits from esid_data as per ISA
+ */
+ get_paca()->slb_cache[slb_cache_index++] = esid_data >> 28;
+ }
+ /*
+ * if we are full, just increment and return.
+ */
+ get_paca()->slb_cache_ptr++;
+}
+
+static void alloc_extended_context(struct mm_struct *mm, unsigned long ea)
+{
+ int context_id;
+
+ int index = ea >> H_BITS_FIRST_CONTEXT;
+
+ /*
+ * we need to do locking only here. If this value was not set before
+ * we will have taken an SLB miss and will reach here. The value will
+ * be either 0 or a valid extended context. We need to make sure two
+ * parallel SLB miss don't end up allocating extended_context for the
+ * same range. The locking below ensures that. For now we take the
+ * heavy mmap_sem. But can be changed to per mm_context_t custom lock
+ * if needed.
+ */
+ down_read(&mm->mmap_sem);
+ context_id = hash__alloc_context_id();
+ if (context_id < 0) {
+ up_read(&mm->mmap_sem);
+ pagefault_out_of_memory();
+ return;
+ }
+ /* Check for parallel allocation after holding lock */
+ if (!mm->context.extended_id[index])
+ mm->context.extended_id[index] = context_id;
+ else
+ __destroy_context(context_id);
+ up_read(&mm->mmap_sem);
+}
+
+static void __handle_multi_context_slb_miss(struct pt_regs *regs,
+ unsigned long ea)
+{
+ int context, bpsize;
+ unsigned long vsid;
+ struct mm_struct *mm = current->mm;
+
+ context = get_esid_context(&mm->context, ea);
+ if (!context) {
+ /*
+ * haven't allocated context yet for this range.
+ * Enable irq and allo context and return. We will
+ * take an slb miss on this again and come here with
+ * allocated context.
+ */
+ /* We restore the interrupt state now */
+ if (!arch_irq_disabled_regs(regs))
+ local_irq_enable();
+ return alloc_extended_context(mm, ea);
+ }
+ /*
+ * We are always above 1TB, hence use high user segment size.
+ */
+ vsid = __get_vsid(context, ea, mmu_highuser_ssize);
+ bpsize = get_slice_psize(mm, ea);
+
+ insert_slb_entry(vsid, ea, bpsize, mmu_highuser_ssize);
+}
+
+/*
+ * exception_enter() handling? FIXME!!
+ */
+void handle_multi_context_slb_miss(struct pt_regs *regs)
+{
+ enum ctx_state prev_state = exception_enter();
+ unsigned long ea = regs->dar;
+
+ /*
+ * Kernel always runs with single context. Hence
+ * anything that request for multi context is
+ * considered bad slb request.
+ */
+ if (!user_mode(regs))
+ return bad_page_fault(regs, ea, SIGSEGV);
+
+ if (REGION_ID(ea) != USER_REGION_ID)
+ goto slb_bad_addr;
+ /*
+ * Are we beyound what the page table layout support ?
+ */
+ if ((ea & ~REGION_MASK) >= H_PGTABLE_RANGE)
+ goto slb_bad_addr;
+
+#ifdef CONFIG_PPC_MM_SLICES
+ /*
+ * consider this as bad slb if we take an slb miss
+ * on an address above addr limit.
+ */
+ if (ea >= current->mm->context.slb_addr_limit)
+ goto slb_bad_addr;
+#endif
+ /* Lower address should be handled by asm code */
+ if (ea < (1UL << H_BITS_FIRST_CONTEXT))
+ goto slb_bad_addr;
+
+ __handle_multi_context_slb_miss(regs, ea);
+ exception_exit(prev_state);
+ return;
+
+slb_bad_addr:
+ _exception(SIGSEGV, regs, SEGV_BNDERR, ea);
+ exception_exit(prev_state);
+}
diff --git a/arch/powerpc/mm/slb_low.S b/arch/powerpc/mm/slb_low.S
index 2c7c717fd2ea..c66cb06e73a1 100644
--- a/arch/powerpc/mm/slb_low.S
+++ b/arch/powerpc/mm/slb_low.S
@@ -75,10 +75,12 @@ ALT_MMU_FTR_SECTION_END_IFCLR(MMU_FTR_68_BIT_VA)
*/
_GLOBAL(slb_allocate)
/*
- * check for bad kernel/user address
+ * Check for address range for which we need to handle multi context. For
+ * the default context we allocate the slb via the fast path. For large
+ * address we branch out to C-code and look at additional context allocated.
* (ea & ~REGION_MASK) >= PGTABLE_RANGE
*/
- rldicr. r9,r3,4,(63 - H_PGTABLE_EADDR_SIZE - 4)
+ rldicr. r9,r3,4,(63 - H_BITS_FIRST_CONTEXT - 4)
bne- 8f
srdi r9,r3,60 /* get region */
diff --git a/arch/powerpc/mm/tlb_hash64.c b/arch/powerpc/mm/tlb_hash64.c
index 9b23f12e863c..87d71dd25441 100644
--- a/arch/powerpc/mm/tlb_hash64.c
+++ b/arch/powerpc/mm/tlb_hash64.c
@@ -89,7 +89,7 @@ void hpte_need_flush(struct mm_struct *mm, unsigned long addr,
/* Build full vaddr */
if (!is_kernel_addr(addr)) {
ssize = user_segment_size(addr);
- vsid = get_vsid(mm->context.id, addr, ssize);
+ vsid = get_user_vsid(&mm->context, addr, ssize);
} else {
vsid = get_kernel_vsid(addr, mmu_kernel_ssize);
ssize = mmu_kernel_ssize;
--
2.14.3
^ permalink raw reply related
* Re: [PATCH 00/14] numa aware allocation for pacas, stacks, pagetables
From: Nicholas Piggin @ 2018-03-07 11:23 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <87muzkdusj.fsf@concordia.ellerman.id.au>
On Wed, 07 Mar 2018 21:50:04 +1100
Michael Ellerman <mpe@ellerman.id.au> wrote:
> Nicholas Piggin <npiggin@gmail.com> writes:
>
> > This series allows numa aware allocations for various early data
> > structures for radix. Hash still has a bolted SLB limitation that
> > prevents at least pacas and stacks from node-affine allocations.
> >
> > Fixed up a number of bugs, got pSeries working, added a couple more
> > cases where page tables can be allocated node-local.
>
> Few problems in here:
>
> FAILURE kernel-build-linux » powerpc,gcc_ubuntu_be,pmac32
> arch/powerpc/kernel/prom.c:748:2: error: implicit declaration of function 'allocate_paca_ptrs' [-Werror=implicit-function-declaration]
>
> FAILURE kernel-build-linux » powerpc,gcc_ubuntu_le,powernv
> arch/powerpc/include/asm/paca.h:49:33: error: 'struct paca_struct' has no member named 'lppaca_ptr'
> arch/powerpc/include/asm/paca.h:49:33: error: 'struct paca_struct' has no member named 'lppaca_ptr'
>
> Did I miss a follow-up or something?
No, I probably just don't do enough compile testing on ppc32. Not
sure about the powernv error, probably just missed testing a config.
Do you have more logs?
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH 00/14] numa aware allocation for pacas, stacks, pagetables
From: Michael Ellerman @ 2018-03-07 10:50 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: Nicholas Piggin
In-Reply-To: <20180213150824.27689-1-npiggin@gmail.com>
Nicholas Piggin <npiggin@gmail.com> writes:
> This series allows numa aware allocations for various early data
> structures for radix. Hash still has a bolted SLB limitation that
> prevents at least pacas and stacks from node-affine allocations.
>
> Fixed up a number of bugs, got pSeries working, added a couple more
> cases where page tables can be allocated node-local.
Few problems in here:
FAILURE kernel-build-linux =C2=BB powerpc,gcc_ubuntu_be,pmac32
arch/powerpc/kernel/prom.c:748:2: error: implicit declaration of function=
'allocate_paca_ptrs' [-Werror=3Dimplicit-function-declaration]
FAILURE kernel-build-linux =C2=BB powerpc,gcc_ubuntu_le,powernv
arch/powerpc/include/asm/paca.h:49:33: error: 'struct paca_struct' has no=
member named 'lppaca_ptr'
arch/powerpc/include/asm/paca.h:49:33: error: 'struct paca_struct' has no=
member named 'lppaca_ptr'
Did I miss a follow-up or something?
cheers
^ permalink raw reply
* Re: [PATCH v2 10/10] powerpc/mm/slice: use the dynamic high slice size to limit bitmap operations
From: Michael Ellerman @ 2018-03-07 10:45 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev; +Cc: Aneesh Kumar K . V
In-Reply-To: <20180307132232.4fe41c3b@roar.ozlabs.ibm.com>
Nicholas Piggin <npiggin@gmail.com> writes:
> On Wed, 7 Mar 2018 11:37:18 +1000
> Nicholas Piggin <npiggin@gmail.com> wrote:
>
>> The number of high slices a process might use now depends on its
>> address space size, and what allocation address it has requested.
>>
>> This patch uses that limit throughout call chains where possible,
>> rather than use the fixed SLICE_NUM_HIGH for bitmap operations.
>> This saves some cost for processes that don't use very large address
>> spaces.
>>
>> Perormance numbers aren't changed significantly, this may change
>> with larger address spaces or different mmap access patterns that
>> require more slice mask building.
>
>
> Ignore this patch in the series. I didn't intend to send it.
Oops :D
I'll drop it and rebuild.
cheers
kisskb: Failed 206/268
http://kisskb.ellerman.id.au/kisskb/head/82bc47f26969f6ed290cda529b9893941923c0f4/
Failed: powerpc-next/pseries_le_defconfig+NO_NUMA/ppc64le (http://kisskb.ellerman.id.au/kisskb/buildresult/13296349/log/)
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/pseries_le_defconfig+NO_SPLPAR/ppc64le (http://kisskb.ellerman.id.au/kisskb/buildresult/13296347/log/)
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/pseries_defconfig+NO_SPLPAR/powerpc-5.3 (http://kisskb.ellerman.id.au/kisskb/buildresult/13296346/log/)
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/pseries_defconfig+NO_SPLPAR/powerpc (http://kisskb.ellerman.id.au/kisskb/buildresult/13296345/log/)
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/pmac32_defconfig+KVM/powerpc (http://kisskb.ellerman.id.au/kisskb/buildresult/13296343/log/)
/kisskb/src/arch/powerpc/kvm/powerpc.c:1361:2: error: 'emulated' may be used uninitialized in this function [-Werror=uninitialized]
Failed: powerpc-next/allmodconfig+64K_PAGES/powerpc-5.3 (http://kisskb.ellerman.id.au/kisskb/buildresult/13296342/log/)
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/allmodconfig+64K_PAGES/powerpc (http://kisskb.ellerman.id.au/kisskb/buildresult/13296341/log/)
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/skiroot_defconfig/ppc64le (http://kisskb.ellerman.id.au/kisskb/buildresult/13296339/log/)
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/powernv_defconfig+NO_NUMA/ppc64le (http://kisskb.ellerman.id.au/kisskb/buildresult/13296338/log/)
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/powernv_defconfig+STRICT_RWX/ppc64le (http://kisskb.ellerman.id.au/kisskb/buildresult/13296337/log/)
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/corenet64_smp_defconfig/powerpc-5.3 (http://kisskb.ellerman.id.au/kisskb/buildresult/13296332/log/)
/kisskb/src/arch/powerpc/include/asm/hugetlb.h:101:10: error: implicit declaration of function 'slice_is_hugepage_only_range' [-Werror=implicit-function-declaration]
Failed: powerpc-next/corenet32_smp_defconfig/powerpc-5.3 (http://kisskb.ellerman.id.au/kisskb/buildresult/13296331/log/)
/kisskb/src/arch/powerpc/include/asm/hugetlb.h:101:10: error: implicit declaration of function 'slice_is_hugepage_only_range' [-Werror=implicit-function-declaration]
Failed: powerpc-next/ppc64_defconfig+NO_HUGETLB/powerpc-5.3 (http://kisskb.ellerman.id.au/kisskb/buildresult/13296322/log/)
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/ppc64_defconfig+NO_KVM/powerpc-5.3 (http://kisskb.ellerman.id.au/kisskb/buildresult/13296321/log/)
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/pseries_defconfig+NO_MEMORY_HOTPLUG/powerpc-5.3 (http://kisskb.ellerman.id.au/kisskb/buildresult/13296319/log/)
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/powerpc-randconfig/powerpc-5.3 (http://kisskb.ellerman.id.au/kisskb/buildresult/13296317/log/)
/kisskb/src/arch/powerpc/kernel/watchdog.c:168:3: error: implicit declaration of function 'smp_flush_nmi_ipi' [-Werror=implicit-function-declaration]
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/kernel/watchdog.c:166:4: error: implicit declaration of function 'smp_send_nmi_ipi' [-Werror=implicit-function-declaration]
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/ppc64_defconfig+NO_TM/powerpc-5.3 (http://kisskb.ellerman.id.au/kisskb/buildresult/13296316/log/)
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/pseries_defconfig+NO_MEMORY_HOTREMOVE/powerpc (http://kisskb.ellerman.id.au/kisskb/buildresult/13296292/log/)
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/mpc85xx_smp_defconfig/powerpc-5.3 (http://kisskb.ellerman.id.au/kisskb/buildresult/13296256/log/)
/kisskb/src/arch/powerpc/include/asm/hugetlb.h:101:10: error: implicit declaration of function 'slice_is_hugepage_only_range' [-Werror=implicit-function-declaration]
Failed: powerpc-next/ppc64_defconfig+UP/powerpc-5.3 (http://kisskb.ellerman.id.au/kisskb/buildresult/13296248/log/)
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/ppc64_defconfig+NO_ALTIVEC/powerpc-5.3 (http://kisskb.ellerman.id.au/kisskb/buildresult/13296247/log/)
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/powerpc-allyesconfig/powerpc-5.3 (http://kisskb.ellerman.id.au/kisskb/buildresult/13296246/log/)
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/powerpc-allmodconfig/powerpc-5.3 (http://kisskb.ellerman.id.au/kisskb/buildresult/13296243/log/)
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/ppc64_defconfig/powerpc-5.3 (http://kisskb.ellerman.id.au/kisskb/buildresult/13296239/log/)
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/mpc85xx_defconfig/powerpc-5.3 (http://kisskb.ellerman.id.au/kisskb/buildresult/13296233/log/)
/kisskb/src/arch/powerpc/include/asm/hugetlb.h:101:10: error: implicit declaration of function 'slice_is_hugepage_only_range' [-Werror=implicit-function-declaration]
Failed: powerpc-next/pasemi_defconfig/powerpc-5.3 (http://kisskb.ellerman.id.au/kisskb/buildresult/13296230/log/)
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/ps3_defconfig/powerpc-5.3 (http://kisskb.ellerman.id.au/kisskb/buildresult/13296229/log/)
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/maple_defconfig/powerpc-5.3 (http://kisskb.ellerman.id.au/kisskb/buildresult/13296228/log/)
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/g5_defconfig/powerpc-5.3 (http://kisskb.ellerman.id.au/kisskb/buildresult/13296227/log/)
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/cell_defconfig/powerpc-5.3 (http://kisskb.ellerman.id.au/kisskb/buildresult/13296225/log/)
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/pseries_defconfig/powerpc-5.3 (http://kisskb.ellerman.id.au/kisskb/buildresult/13296222/log/)
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/powernv_defconfig/ppc64le (http://kisskb.ellerman.id.au/kisskb/buildresult/13296221/log/)
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/ppc64_defconfig+NO_ALTIVEC/powerpc (http://kisskb.ellerman.id.au/kisskb/buildresult/13296219/log/)
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
/kisskb/src/arch/powerpc/kvm/powerpc.c:1611:1: error: label 'out' defined but not used [-Werror=unused-label]
Failed: powerpc-next/corenet64_smp_defconfig/powerpc (http://kisskb.ellerman.id.au/kisskb/buildresult/13296213/log/)
/kisskb/src/arch/powerpc/include/asm/hugetlb.h:101:3: error: implicit declaration of function 'slice_is_hugepage_only_range' [-Werror=implicit-function-declaration]
Failed: powerpc-next/corenet32_smp_defconfig/powerpc (http://kisskb.ellerman.id.au/kisskb/buildresult/13296212/log/)
/kisskb/src/arch/powerpc/include/asm/hugetlb.h:101:3: error: implicit declaration of function 'slice_is_hugepage_only_range' [-Werror=implicit-function-declaration]
Failed: powerpc-next/pseries_defconfig+FA_DUMP/powerpc-5.3 (http://kisskb.ellerman.id.au/kisskb/buildresult/13296201/log/)
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/ppc64_defconfig+NO_HUGETLB/powerpc (http://kisskb.ellerman.id.au/kisskb/buildresult/13296200/log/)
/kisskb/src/arch/powerpc/kvm/powerpc.c:1361:2: error: 'emulated' may be used uninitialized in this function [-Werror=uninitialized]
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/allmodconfig+ppc64le/ppc64le (http://kisskb.ellerman.id.au/kisskb/buildresult/13296199/log/)
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/pseries_defconfig+FA_DUMP/powerpc (http://kisskb.ellerman.id.au/kisskb/buildresult/13296198/log/)
/kisskb/src/arch/powerpc/kvm/powerpc.c:1361:2: error: 'emulated' may be used uninitialized in this function [-Werror=uninitialized]
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/ppc64le_defconfig+NO_KVM/ppc64le (http://kisskb.ellerman.id.au/kisskb/buildresult/13296197/log/)
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/ppc64_defconfig+NO_KVM/powerpc (http://kisskb.ellerman.id.au/kisskb/buildresult/13296196/log/)
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/pseries_le_defconfig/ppc64le (http://kisskb.ellerman.id.au/kisskb/buildresult/13296195/log/)
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/ppc64le_defconfig/ppc64le (http://kisskb.ellerman.id.au/kisskb/buildresult/13296194/log/)
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/ppc64_defconfig+NO_TM/powerpc (http://kisskb.ellerman.id.au/kisskb/buildresult/13296193/log/)
/kisskb/src/arch/powerpc/kvm/powerpc.c:1361:2: error: 'emulated' may be used uninitialized in this function [-Werror=uninitialized]
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/ppc64_defconfig+UP/powerpc (http://kisskb.ellerman.id.au/kisskb/buildresult/13296181/log/)
/kisskb/src/arch/powerpc/kvm/powerpc.c:1361:2: error: 'emulated' may be used uninitialized in this function [-Werror=uninitialized]
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/ppc64_defconfig+NO_RADIX/powerpc (http://kisskb.ellerman.id.au/kisskb/buildresult/13296177/log/)
/kisskb/src/arch/powerpc/kvm/powerpc.c:1361:2: error: 'emulated' may be used uninitialized in this function [-Werror=uninitialized]
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/pseries_defconfig+NO_MEMORY_HOTPLUG/powerpc (http://kisskb.ellerman.id.au/kisskb/buildresult/13296175/log/)
/kisskb/src/arch/powerpc/kvm/powerpc.c:1361:2: error: 'emulated' may be used uninitialized in this function [-Werror=uninitialized]
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/powernv_defconfig+NO_RADIX/ppc64le (http://kisskb.ellerman.id.au/kisskb/buildresult/13296173/log/)
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/maple_defconfig/powerpc (http://kisskb.ellerman.id.au/kisskb/buildresult/13296170/log/)
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/mpc85xx_smp_defconfig/powerpc (http://kisskb.ellerman.id.au/kisskb/buildresult/13296120/log/)
/kisskb/src/arch/powerpc/include/asm/hugetlb.h:101:3: error: implicit declaration of function 'slice_is_hugepage_only_range' [-Werror=implicit-function-declaration]
Failed: powerpc-next/powerpc-randconfig/powerpc (http://kisskb.ellerman.id.au/kisskb/buildresult/13296110/log/)
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/mpc85xx_defconfig/powerpc (http://kisskb.ellerman.id.au/kisskb/buildresult/13296105/log/)
/kisskb/src/arch/powerpc/include/asm/hugetlb.h:101:3: error: implicit declaration of function 'slice_is_hugepage_only_range' [-Werror=implicit-function-declaration]
Failed: powerpc-next/powerpc-allyesconfig/powerpc (http://kisskb.ellerman.id.au/kisskb/buildresult/13296104/log/)
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/powerpc-allmodconfig/powerpc (http://kisskb.ellerman.id.au/kisskb/buildresult/13296103/log/)
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/pasemi_defconfig/powerpc (http://kisskb.ellerman.id.au/kisskb/buildresult/13296101/log/)
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/ppc64_defconfig/powerpc (http://kisskb.ellerman.id.au/kisskb/buildresult/13296100/log/)
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/ps3_defconfig/powerpc (http://kisskb.ellerman.id.au/kisskb/buildresult/13296096/log/)
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/g5_defconfig/powerpc (http://kisskb.ellerman.id.au/kisskb/buildresult/13296095/log/)
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/cell_defconfig/powerpc (http://kisskb.ellerman.id.au/kisskb/buildresult/13296093/log/)
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
Failed: powerpc-next/pseries_defconfig/powerpc (http://kisskb.ellerman.id.au/kisskb/buildresult/13296091/log/)
/kisskb/src/arch/powerpc/kvm/powerpc.c:1361:2: error: 'emulated' may be used uninitialized in this function [-Werror=uninitialized]
/kisskb/src/arch/powerpc/mm/slice.c:81:2: error: #error XXX: bitmap_zero with non-const size? Good code?
/kisskb/src/arch/powerpc/mm/slice.c:660:2: error: #error XXX: are we sure high_slices is > 0 here when SLICE_NUM_HIGH? Perhaps not for 32-bit tasks on 64?
^ permalink raw reply
* [PATCH] PCI/hotplug: ppc: correct a php_slot usage after free
From: wei.guo.simon @ 2018-03-07 8:46 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Benjamin Herrenschmidt, linux-pci, linux-kernel, Simon Guo
From: Simon Guo <wei.guo.simon@gmail.com>
In pnv_php_unregister_one(), pnv_php_put_slot() might kfree
php_slot structure. But there is pci_hp_deregister() after
that with php_slot reference.
This patch moves pnv_php_put_slot() to the end of function.
Signed-off-by: Simon Guo <wei.guo.simon@gmail.com>
---
drivers/pci/hotplug/pnv_php.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pci/hotplug/pnv_php.c b/drivers/pci/hotplug/pnv_php.c
index 74f6a17..eb60692e 100644
--- a/drivers/pci/hotplug/pnv_php.c
+++ b/drivers/pci/hotplug/pnv_php.c
@@ -930,8 +930,8 @@ static void pnv_php_unregister_one(struct device_node *dn)
return;
php_slot->state = PNV_PHP_STATE_OFFLINE;
- pnv_php_put_slot(php_slot);
pci_hp_deregister(&php_slot->slot);
+ pnv_php_put_slot(php_slot);
}
static void pnv_php_unregister(struct device_node *dn)
--
1.8.3.1
^ permalink raw reply related
* Re: [RESEND][PATCH] cpuidle/powernv : Restore different PSSCR for idle and hotplug
From: Vaidyanathan Srinivasan @ 2018-03-07 7:56 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Akshay Adiga, linux-kernel, linuxppc-dev, npiggin, stable
In-Reply-To: <1519854022.2520.12.camel@au1.ibm.com>
* Benjamin Herrenschmidt <benh@au1.ibm.com> [2018-03-01 08:40:22]:
> On Thu, 2018-03-01 at 01:03 +0530, Akshay Adiga wrote:
> > commit 1e1601b38e6e ("powerpc/powernv/idle: Restore SPRs for deep idle
> > states via stop API.") uses stop-api provided by the firmware to restore
> > PSSCR. PSSCR restore is required for handling special wakeup. When special
> > wakeup is completed, the core enters stop state based on restored PSSCR.
> >
> > Currently PSSCR is restored to deepest available stop state, causing
> > a idle cpu to enter deeper stop state on a special wakeup, which causes
> > the cpu to hang on wakeup.
> >
> > A "sensors" command which reads temperature (through DTS sensors) on idle
> > cpu can trigger special wakeup.
> >
> > Failed Scenario :
> > Request restore of PSSCR with RL = 11
> > cpu enters idle state (stop5)
> > user triggers "sensors" command
> > Assert special wakeup on cpu
> > Restores PSSCR with RL = 11 <---- Done by firmware
> > Read DTS sensor
> > Deassert special wakeup
> > cpu enters idle state (stop11) <-- Instead of stop5
> >
> > Cpu hang is caused because cpu ended up in a deeper state than it requested
> >
> > This patch fixes instability caused by special wakeup when stop11 is
> > enabled. Requests restore of PSSCR to deepest stop state used by cpuidle.
> > Only when offlining cpu, request restore of PSSCR to deepest stop state.
> > On onlining cpu, request restore of PSSCR to deepest stop state used by
> > cpuidle.
>
> So if we chose a stop state, but somebody else does a special wakeup,
> we'll end up going back into a *deeper* one than the one we came from ?
Unfortunately yes. This is the current limitation. If we are in stop4
and above and we had not set a PSSCR to be restored, then the hardware
will default to all bits set (stop15) leading to entry into stop11
after the special wakeup is removed. The requirement is such that we
need to have a correct PSSCR restore value set using stop-api.
We need to set a restore PSSCR value that represents one in a group
like stop4,5,6,7 will have identical state loss, hence we can either
set a PSSCR of 7 or 4 or 5 for any of this stop state entry and not
have to use stop-api to set exact value of stop4 or 5 at every entry.
> I still think this is broken by design. The chip should automatically
> go back to the state it went to after special wakeup, thus the PPE
> controlling the state should override the PSSCR value accordingly
> rather than relying on those SW hoops.
Special wakeup de-assertion and re-entry hits this limitation where we
have lost the original content of PSSCR SPR and hence CME does not know
what was requested.
Additional stop-api calls from software could have been avoided, but
practically we have only cpu hotplug case that uses stop11 and needs
this stop-api. We can default the system to stop4 or stop5 and then
make stop-api call to explicitly set stop11 and then hotplug out the
cpu. We have to restore the deepest cpuidle state (stop4/5) back
during online. As such this is not much of software overhead. But we
need an elegant method to control these calls from OPAL flags so that
kernel behaviour can be more closely controlled.
If we want to use stop11 in cpuidle (despite being very slow) for
evaluation reasons, then we will need to make more stop-api call to
choose between stop4/5 vs stop11 since they belong to different group.
Even in this case, since stop11 is the slow path, we would want to set
stop11 before entry and restore to stop4/5 after wakeup. This way we
still completely avoid stop-api call in fast-path stop4/5 entry/exit.
--Vaidy
^ permalink raw reply
* Re: [PATCH 06/10] powerpc/mm/slice: implement slice_check_range_fits
From: Nicholas Piggin @ 2018-03-07 7:16 UTC (permalink / raw)
To: Christophe LEROY; +Cc: linuxppc-dev, Aneesh Kumar K . V
In-Reply-To: <a61d33eb-dde3-526a-0847-e2d56f824217@c-s.fr>
On Wed, 7 Mar 2018 07:12:23 +0100
Christophe LEROY <christophe.leroy@c-s.fr> wrote:
> Le 07/03/2018 à 00:12, Nicholas Piggin a écrit :
> > On Tue, 6 Mar 2018 15:41:00 +0100
> > Christophe LEROY <christophe.leroy@c-s.fr> wrote:
> >
> >> Le 06/03/2018 à 14:25, Nicholas Piggin a écrit :
> >>> @@ -596,10 +601,11 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
> >>> slice_or_mask(&potential_mask, &good_mask);
> >>> slice_print_mask(" potential", &potential_mask);
> >>>
> >>> - if ((addr != 0 || fixed) &&
> >>> - slice_check_fit(mm, &mask, &potential_mask)) {
> >>> - slice_dbg(" fits potential !\n");
> >>> - goto convert;
> >>> + if (addr || fixed) {
> >>> + if (slice_check_range_fits(mm, &potential_mask, addr, len)) {
> >>> + slice_dbg(" fits potential !\n");
> >>> + goto convert;
> >>> + }
> >>
> >> Why not keep the original structure and just replacing slice_check_fit()
> >> by slice_check_range_fits() ?
> >>
> >> I believe cleanups should not be mixed with real feature changes. If
> >> needed, you should have a cleanup patch up front the serie.
> >
> > For code that is already changing, I think minor cleanups are okay if
> > they're very simple. Maybe this is getting to the point of needing
> > another patch. You've made valid points for a lot of other unnecessary
> > cleanups though, so I'll fix all of those.
>
> Ok, that's not a big point, but I like when patches really modifies
> only the lines they need to modify.
Fair point, and in the end I agree mostly they should do that. But I
don't think entirely if you can make the code slightly better as you
go (again, so long as the change is obvious). I think having extra
patches for trivial cleanups is not that great either.
> Why do we need a double if ?
>
> Why not just the following ? With proper alignment of the second line
> with the open parenthese, it fits in one line
>
> if ((addr != 0 || fixed) &&
> - slice_check_fit(mm, &mask, &potential_mask)) {
> + slice_check_range_fits(mm, &potential_mask, addr, len)) {
> slice_dbg(" fits potential !\n");
> goto convert;
For this case the main motivation was to make this test match the
form of the same test (with different mask) above here. Doing the
same thing with different coding styles annoys me.
I think I kept this one but fixed all your other suggestions in
the v2 series.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH v2 06/38] cxlflash: Read host function configuration
From: Andrew Donnellan @ 2018-03-07 6:37 UTC (permalink / raw)
To: Uma Krishnan, linux-scsi, James Bottomley, Martin K. Petersen,
Matthew R. Ochs, Manoj N. Kumar
Cc: linuxppc-dev, Frederic Barrat, Christophe Lombard
In-Reply-To: <1519683641-16967-1-git-send-email-ukrishn@linux.vnet.ibm.com>
On 27/02/18 09:20, Uma Krishnan wrote:
> Per the OCXL specification, the underlying host can have multiple AFUs
> per function with each function supporting its own configuration. The host
> function configuration is read on the initialization path to evaluate the
> number of functions present and identify the features and configuration of
> the functions present. This data is cached for use in later configuration
> steps. Note that for the OCXL hardware supported by the cxlflash driver,
> only one AFU per function is expected.
>
> Signed-off-by: Uma Krishnan <ukrishn@linux.vnet.ibm.com>
> Acked-by: Matthew R. Ochs <mrochs@linux.vnet.ibm.com>
Nitpick below
Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
> ---
> drivers/scsi/cxlflash/ocxl_hw.c | 41 +++++++++++++++++++++++++++++++++++++++++
> drivers/scsi/cxlflash/ocxl_hw.h | 2 ++
> 2 files changed, 43 insertions(+)
>
> diff --git a/drivers/scsi/cxlflash/ocxl_hw.c b/drivers/scsi/cxlflash/ocxl_hw.c
> index e3a0a9b..dc32a73 100644
> --- a/drivers/scsi/cxlflash/ocxl_hw.c
> +++ b/drivers/scsi/cxlflash/ocxl_hw.c
> @@ -32,6 +32,35 @@ static void ocxlflash_destroy_afu(void *afu_cookie)
> }
>
> /**
> + * ocxlflash_config_fn() - configure the host function
> + * @pdev: PCI device associated with the host.
> + * @afu: AFU associated with the host.
> + *
> + * Return: 0 on success, -errno on failure
> + */
> +static int ocxlflash_config_fn(struct pci_dev *pdev, struct ocxl_hw_afu *afu)
> +{
> + struct ocxl_fn_config *fcfg = &afu->fcfg;
> + struct device *dev = &pdev->dev;
> + int rc = 0;
> +
> + /* Read DVSEC config of the function */
> + rc = ocxl_config_read_function(pdev, fcfg);
> + if (unlikely(rc)) {
> + dev_err(dev, "%s: ocxl_config_read_function failed rc=%d\n",
> + __func__, rc);
> + goto out;
> + }
> +
> + /* Only one AFU per function is supported by ocxlflash */
> + if (fcfg->max_afu_index != 0)
> + dev_warn(dev, "%s: Unexpected AFU index value %d\n",
> + __func__, fcfg->max_afu_index);
> +out:
> + return rc;
> +}
> +
> +/**
> * ocxlflash_create_afu() - create the AFU for OCXL
> * @pdev: PCI device associated with the host.
> *
> @@ -41,6 +70,7 @@ static void *ocxlflash_create_afu(struct pci_dev *pdev)
> {
> struct device *dev = &pdev->dev;
> struct ocxl_hw_afu *afu;
> + int rc;
>
> afu = kzalloc(sizeof(*afu), GFP_KERNEL);
> if (unlikely(!afu)) {
> @@ -50,8 +80,19 @@ static void *ocxlflash_create_afu(struct pci_dev *pdev)
>
> afu->pdev = pdev;
> afu->dev = dev;
> +
> + rc = ocxlflash_config_fn(pdev, afu);
> + if (unlikely(rc)) {
> + dev_err(dev, "%s: Function configuration failed rc=%d\n",
> + __func__, rc);
> + goto err1;
> + }
> out:
> return afu;
> +err1:
> + kfree(afu);
> + afu = NULL;
> + goto out;
I think it would be cleaner to just write return NULL here
> }
>
> /* Backend ops to ocxlflash services */
> diff --git a/drivers/scsi/cxlflash/ocxl_hw.h b/drivers/scsi/cxlflash/ocxl_hw.h
> index c7e5c4d..658f420 100644
> --- a/drivers/scsi/cxlflash/ocxl_hw.h
> +++ b/drivers/scsi/cxlflash/ocxl_hw.h
> @@ -16,4 +16,6 @@
> struct ocxl_hw_afu {
> struct pci_dev *pdev; /* PCI device */
> struct device *dev; /* Generic device */
> +
> + struct ocxl_fn_config fcfg; /* DVSEC config of the function */
> };
>
--
Andrew Donnellan OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com IBM Australia Limited
^ permalink raw reply
* Re: [PATCH 06/10] powerpc/mm/slice: implement slice_check_range_fits
From: Christophe LEROY @ 2018-03-07 6:12 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: linuxppc-dev, Aneesh Kumar K . V
In-Reply-To: <20180307091232.4fb8d3c2@roar.ozlabs.ibm.com>
Le 07/03/2018 à 00:12, Nicholas Piggin a écrit :
> On Tue, 6 Mar 2018 15:41:00 +0100
> Christophe LEROY <christophe.leroy@c-s.fr> wrote:
>
>> Le 06/03/2018 à 14:25, Nicholas Piggin a écrit :
>
>
>>> +static bool slice_check_range_fits(struct mm_struct *mm,
>>> + const struct slice_mask *available,
>>> + unsigned long start, unsigned long len)
>>> {
>>> - DECLARE_BITMAP(result, SLICE_NUM_HIGH);
>>> - /*
>>> - * Make sure we just do bit compare only to the max
>>> - * addr limit and not the full bit map size.
>>> - */
>>> - unsigned long slice_count = GET_HIGH_SLICE_INDEX(mm->context.slb_addr_limit);
>>> + unsigned long end = start + len - 1;
>>> + u64 low_slices = 0;
>>>
>>> - if (!SLICE_NUM_HIGH)
>>> - return (mask->low_slices & available->low_slices) ==
>>> - mask->low_slices;
>>> + if (start < SLICE_LOW_TOP) {
>>> + unsigned long mend = min(end, (SLICE_LOW_TOP - 1));
>>
>> See slice_range_to_mask()
>>
>> You'll have an issue here with PPC32, you have to cast (SLICE_LOW_TOP -
>> 1) to unsigned long because SLICE_LOW_TOP is unsigned long long on PPC32
>
> Okay thanks. Forgot to cross compiled it on 8xx, so I'll do that next
> time.
>
>>> +
>>> + low_slices = (1u << (GET_LOW_SLICE_INDEX(mend) + 1))
>>> + - (1u << GET_LOW_SLICE_INDEX(start));
>>> + }
>>> + if ((low_slices & available->low_slices) != low_slices)
>>> + return false;
>>> +
>>> + if (SLICE_NUM_HIGH && ((start + len) > SLICE_LOW_TOP)) {
>>> + unsigned long start_index = GET_HIGH_SLICE_INDEX(start);
>>> + unsigned long align_end = ALIGN(end, (1UL << SLICE_HIGH_SHIFT));
>>> + unsigned long count = GET_HIGH_SLICE_INDEX(align_end) - start_index;
>>> + unsigned long i;
>>>
>>> - bitmap_and(result, mask->high_slices,
>>> - available->high_slices, slice_count);
>>> + for (i = start_index; i < start_index + count; i++) {
>>> + if (!test_bit(i, available->high_slices))
>>> + return false;
>>> + }
>>
>> What about using bitmap_find_next_zero_area()
>
> I'll look at it. Perhaps in another patch, because existing
> loops are not using bitmap range operations either. A series
> to convert those is a good idea.
>
>>> @@ -562,15 +571,11 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
>>> #endif
>>>
>>> /* First check hint if it's valid or if we have MAP_FIXED */
>>> - if (addr != 0 || fixed) {
>>> - /* Build a mask for the requested range */
>>> - slice_range_to_mask(addr, len, &mask);
>>> - slice_print_mask(" mask", &mask);
>>> -
>>> + if (addr || fixed) {
>>
>> It is cleanup, should it really be part of this patch ?
>
>
>
>>> @@ -596,10 +601,11 @@ unsigned long slice_get_unmapped_area(unsigned long addr, unsigned long len,
>>> slice_or_mask(&potential_mask, &good_mask);
>>> slice_print_mask(" potential", &potential_mask);
>>>
>>> - if ((addr != 0 || fixed) &&
>>> - slice_check_fit(mm, &mask, &potential_mask)) {
>>> - slice_dbg(" fits potential !\n");
>>> - goto convert;
>>> + if (addr || fixed) {
>>> + if (slice_check_range_fits(mm, &potential_mask, addr, len)) {
>>> + slice_dbg(" fits potential !\n");
>>> + goto convert;
>>> + }
>>
>> Why not keep the original structure and just replacing slice_check_fit()
>> by slice_check_range_fits() ?
>>
>> I believe cleanups should not be mixed with real feature changes. If
>> needed, you should have a cleanup patch up front the serie.
>
> For code that is already changing, I think minor cleanups are okay if
> they're very simple. Maybe this is getting to the point of needing
> another patch. You've made valid points for a lot of other unnecessary
> cleanups though, so I'll fix all of those.
Ok, that's not a big point, but I like when patches really modifies
only the lines they need to modify. Why do we need a double if ?
Why not just the following ? With proper alignment of the second line
with the open parenthese, it fits in one line
if ((addr != 0 || fixed) &&
- slice_check_fit(mm, &mask, &potential_mask)) {
+ slice_check_range_fits(mm, &potential_mask, addr, len)) {
slice_dbg(" fits potential !\n");
goto convert;
}
Christophe
^ permalink raw reply
* Re: [PATCH v2 05/38] cxlflash: Hardware AFU for OCXL
From: Andrew Donnellan @ 2018-03-07 5:47 UTC (permalink / raw)
To: Uma Krishnan, linux-scsi, James Bottomley, Martin K. Petersen,
Matthew R. Ochs, Manoj N. Kumar
Cc: linuxppc-dev, Frederic Barrat, Christophe Lombard
In-Reply-To: <1519683625-16930-1-git-send-email-ukrishn@linux.vnet.ibm.com>
On 27/02/18 09:20, Uma Krishnan wrote:
> When an adapter is initialized, transport specific configuration and MMIO
> mapping details need to be saved. For CXL, this data is managed by the
> underlying kernel module. To maintain a separation between the cxlflash
> core and underlying transports, introduce a new structure to store data
> specific to the OCXL AFU.
>
> Initially only the pointers to underlying PCI and generic devices are
> added to this new structure - it will be expanded further in future
> commits. Services to create and destroy this hardware AFU are added and
> integrated in the probe and exit paths of the driver.
>
> Signed-off-by: Uma Krishnan <ukrishn@linux.vnet.ibm.com>
> Acked-by: Matthew R. Ochs <mrochs@linux.vnet.ibm.com>
One comment below
Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
> ---
> drivers/scsi/cxlflash/backend.h | 1 +
> drivers/scsi/cxlflash/cxl_hw.c | 6 ++++++
> drivers/scsi/cxlflash/main.c | 9 +++++++--
> drivers/scsi/cxlflash/ocxl_hw.c | 40 ++++++++++++++++++++++++++++++++++++++++
> drivers/scsi/cxlflash/ocxl_hw.h | 19 +++++++++++++++++++
> 5 files changed, 73 insertions(+), 2 deletions(-)
> create mode 100644 drivers/scsi/cxlflash/ocxl_hw.h
>
> diff --git a/drivers/scsi/cxlflash/backend.h b/drivers/scsi/cxlflash/backend.h
> index a60f051..f675bcb 100644
> --- a/drivers/scsi/cxlflash/backend.h
> +++ b/drivers/scsi/cxlflash/backend.h
> @@ -36,6 +36,7 @@ struct cxlflash_backend_ops {
> int (*allocate_afu_irqs)(void *ctx_cookie, int num);
> void (*free_afu_irqs)(void *ctx_cookie);
> void * (*create_afu)(struct pci_dev *dev);
> + void (*destroy_afu)(void *afu_cookie);
> struct file * (*get_fd)(void *ctx_cookie, struct file_operations *fops,
> int *fd);
> void * (*fops_get_context)(struct file *file);
> diff --git a/drivers/scsi/cxlflash/cxl_hw.c b/drivers/scsi/cxlflash/cxl_hw.c
> index db1cada..a1d6d12 100644
> --- a/drivers/scsi/cxlflash/cxl_hw.c
> +++ b/drivers/scsi/cxlflash/cxl_hw.c
> @@ -110,6 +110,11 @@ static void *cxlflash_create_afu(struct pci_dev *dev)
> return cxl_pci_to_afu(dev);
> }
>
> +static void cxlflash_destroy_afu(void *afu)
> +{
> + /* Dummy fop for cxl */
> +}
> +
For ops structs I think it's more common to set the function pointer to
NULL when not implemented and do a NULL check at the call site.
> static struct file *cxlflash_get_fd(void *ctx_cookie,
> struct file_operations *fops, int *fd)
> {
> @@ -160,6 +165,7 @@ const struct cxlflash_backend_ops cxlflash_cxl_ops = {
> .allocate_afu_irqs = cxlflash_allocate_afu_irqs,
> .free_afu_irqs = cxlflash_free_afu_irqs,
> .create_afu = cxlflash_create_afu,
> + .destroy_afu = cxlflash_destroy_afu,
> .get_fd = cxlflash_get_fd,
> .fops_get_context = cxlflash_fops_get_context,
> .start_work = cxlflash_start_work,
> diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c
> index b83a55a..5d754d1 100644
> --- a/drivers/scsi/cxlflash/main.c
> +++ b/drivers/scsi/cxlflash/main.c
> @@ -971,6 +971,7 @@ static void cxlflash_remove(struct pci_dev *pdev)
> case INIT_STATE_AFU:
> term_afu(cfg);
> case INIT_STATE_PCI:
> + cfg->ops->destroy_afu(cfg->afu_cookie);
> pci_disable_device(pdev);
> case INIT_STATE_NONE:
> free_mem(cfg);
> @@ -3689,8 +3690,6 @@ static int cxlflash_probe(struct pci_dev *pdev,
>
> pci_set_drvdata(pdev, cfg);
>
> - cfg->afu_cookie = cfg->ops->create_afu(pdev);
> -
> rc = init_pci(cfg);
> if (rc) {
> dev_err(dev, "%s: init_pci failed rc=%d\n", __func__, rc);
> @@ -3698,6 +3697,12 @@ static int cxlflash_probe(struct pci_dev *pdev,
> }
> cfg->init_state = INIT_STATE_PCI;
>
> + cfg->afu_cookie = cfg->ops->create_afu(pdev);
> + if (unlikely(!cfg->afu_cookie)) {
> + dev_err(dev, "%s: create_afu failed\n", __func__);
> + goto out_remove;
> + }
> +
> rc = init_afu(cfg);
> if (rc && !wq_has_sleeper(&cfg->reset_waitq)) {
> dev_err(dev, "%s: init_afu failed rc=%d\n", __func__, rc);
> diff --git a/drivers/scsi/cxlflash/ocxl_hw.c b/drivers/scsi/cxlflash/ocxl_hw.c
> index 58a3182..e3a0a9b 100644
> --- a/drivers/scsi/cxlflash/ocxl_hw.c
> +++ b/drivers/scsi/cxlflash/ocxl_hw.c
> @@ -15,8 +15,48 @@
> #include <misc/ocxl.h>
>
> #include "backend.h"
> +#include "ocxl_hw.h"
> +
> +/**
> + * ocxlflash_destroy_afu() - destroy the AFU structure
> + * @afu_cookie: AFU to be freed.
> + */
> +static void ocxlflash_destroy_afu(void *afu_cookie)
> +{
> + struct ocxl_hw_afu *afu = afu_cookie;
> +
> + if (!afu)
> + return; > +
> + kfree(afu);
> +}
> +
> +/**
> + * ocxlflash_create_afu() - create the AFU for OCXL
> + * @pdev: PCI device associated with the host.
> + *
> + * Return: AFU on success, NULL on failure
> + */
> +static void *ocxlflash_create_afu(struct pci_dev *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct ocxl_hw_afu *afu;
> +
> + afu = kzalloc(sizeof(*afu), GFP_KERNEL);
> + if (unlikely(!afu)) {
> + dev_err(dev, "%s: HW AFU allocation failed\n", __func__);
> + goto out;
> + }
> +
> + afu->pdev = pdev;
> + afu->dev = dev;
> +out:
> + return afu;
> +}
>
> /* Backend ops to ocxlflash services */
> const struct cxlflash_backend_ops cxlflash_ocxl_ops = {
> .module = THIS_MODULE,
> + .create_afu = ocxlflash_create_afu,
> + .destroy_afu = ocxlflash_destroy_afu,
> };
> diff --git a/drivers/scsi/cxlflash/ocxl_hw.h b/drivers/scsi/cxlflash/ocxl_hw.h
> new file mode 100644
> index 0000000..c7e5c4d
> --- /dev/null
> +++ b/drivers/scsi/cxlflash/ocxl_hw.h
> @@ -0,0 +1,19 @@
> +/*
> + * CXL Flash Device Driver
> + *
> + * Written by: Matthew R. Ochs <mrochs@linux.vnet.ibm.com>, IBM Corporation
> + * Uma Krishnan <ukrishn@linux.vnet.ibm.com>, IBM Corporation
> + *
> + * Copyright (C) 2018 IBM Corporation
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.
> + */
> +
> +/* OCXL hardware AFU associated with the host */
> +struct ocxl_hw_afu {
> + struct pci_dev *pdev; /* PCI device */
> + struct device *dev; /* Generic device */
> +};
>
--
Andrew Donnellan OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com IBM Australia Limited
^ permalink raw reply
* Re: [PATCH] powerpc/perf: Fix kernel address leaks via Sampling registers
From: Madhavan Srinivasan @ 2018-03-07 4:56 UTC (permalink / raw)
Cc: mpe, linuxppc-dev
In-Reply-To: <201803062210.ahTRkTqj%fengguang.wu@intel.com>
On Tuesday 06 March 2018 08:36 PM, kbuild test robot wrote:
> Hi Michael,
>
> I love your patch! Yet something to improve:
>
> [auto build test ERROR on v4.16-rc1]
> [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/Madhavan-Srinivasan/powerpc-perf-Fix-kernel-address-leaks-via-Sampling-registers/20180306-041036
> config: powerpc-pmac32_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
> make.cross ARCH=powerpc
My bad should have added the ppmu->flag or config check to avoid this
in 32bit case. Will respin it.
Maddy
> All errors (new ones prefixed by >>):
>
> {standard input}: Assembler messages:
>>> {standard input}:2476: Error: unsupported relocation against SPRN_SDAR
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply
* Re: [PATCH 1/2] powerpc/perf: Fix kernel address leak to userspace via BHRB buffer
From: Madhavan Srinivasan @ 2018-03-07 4:54 UTC (permalink / raw)
To: Balbir Singh
Cc: Michael Ellerman, open list:LINUX FOR POWERPC (32-BIT AND 64-BIT)
In-Reply-To: <CAKTCnzmY3-n3ZxdUJb_yE_K4sY93j+DfaDVEPBKaGUo_dgbaQg@mail.gmail.com>
On Monday 05 March 2018 11:46 AM, Balbir Singh wrote:
> On Sun, Mar 4, 2018 at 10:55 PM, Madhavan Srinivasan
> <maddy@linux.vnet.ibm.com> wrote:
>> The current Branch History Rolling Buffer (BHRB) code does
>> not check for any privilege levels before updating the data
>> from BHRB. This leaks kernel addresses to userspace even when
>> profiling only with userspace privileges. Add proper checks
>> to prevent it.
>>
>> Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
>> ---
>> arch/powerpc/perf/core-book3s.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
>> index f89bbd54ecec..337db5831749 100644
>> --- a/arch/powerpc/perf/core-book3s.c
>> +++ b/arch/powerpc/perf/core-book3s.c
>> @@ -457,6 +457,10 @@ static void power_pmu_bhrb_read(struct cpu_hw_events *cpuhw)
>> /* invalid entry */
>> continue;
>>
>> + if (perf_paranoid_kernel() && !capable(CAP_SYS_ADMIN) &&
>> + is_kernel_addr(addr))
>> + continue;
>> +
>
> Looks good to me. The scope of the leaks concern is KASLR related or
> something else (figuring out what's in the cache?)
I did not look at it closely. But will get the information.
Thanks for the review
Maddy
>
> Acked-by: Balbir Singh <bsingharora@gmail.com>
>
> Balbir Singh.
>
^ permalink raw reply
* Re: [PATCH 2/2] powerpc/perf: Fix the kernel address leak to userspace via SDAR
From: Madhavan Srinivasan @ 2018-03-07 4:53 UTC (permalink / raw)
To: Naveen N. Rao, mpe; +Cc: linuxppc-dev
In-Reply-To: <1520237954.soawktcmbp.naveen@linux.ibm.com>
On Monday 05 March 2018 01:51 PM, Naveen N. Rao wrote:
> Madhavan Srinivasan wrote:
>> Sampled Data Address Register (SDAR) is a 64-bit
>> register that contains the effective address of
>> the storage operand of an instruction that was
>> being executed, possibly out-of-order, at or around
>> the time that the Performance Monitor alert occurred.
>>
>> In certain scenario SDAR happen to contain the kernel
>> address even for userspace only sampling. Add checks
>> to prevent it.
>>
>> Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
>> ---
>> arch/powerpc/perf/core-book3s.c | 11 ++++++++---
>> 1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/powerpc/perf/core-book3s.c
>> b/arch/powerpc/perf/core-book3s.c
>> index 337db5831749..c4525323d691 100644
>> --- a/arch/powerpc/perf/core-book3s.c
>> +++ b/arch/powerpc/perf/core-book3s.c
>> @@ -95,7 +95,7 @@ static inline unsigned long perf_ip_adjust(struct
>> pt_regs *regs)
>> {
>> return 0;
>> }
>> -static inline void perf_get_data_addr(struct pt_regs *regs, u64
>> *addrp) { }
>> +static inline void perf_get_data_addr(struct pt_regs *regs, u64
>> *addrp, struct perf_event *event) { }
>> static inline u32 perf_get_misc_flags(struct pt_regs *regs)
>> {
>> return 0;
>> @@ -174,7 +174,7 @@ static inline unsigned long perf_ip_adjust(struct
>> pt_regs *regs)
>> * pointed to by SIAR; this is indicated by the
>> [POWER6_]MMCRA_SDSYNC, the
>> * [POWER7P_]MMCRA_SDAR_VALID bit in MMCRA, or the SDAR_VALID bit in
>> SIER.
>> */
>> -static inline void perf_get_data_addr(struct pt_regs *regs, u64 *addrp)
>> +static inline void perf_get_data_addr(struct pt_regs *regs, u64
>> *addrp, struct perf_event *event)
>> {
>> unsigned long mmcra = regs->dsisr;
>> bool sdar_valid;
>> @@ -198,6 +198,11 @@ static inline void perf_get_data_addr(struct
>> pt_regs *regs, u64 *addrp)
>>
>> if (!(mmcra & MMCRA_SAMPLE_ENABLE) || sdar_valid)
>> *addrp = mfspr(SPRN_SDAR);
>> +
>> + if (perf_paranoid_kernel() && !capable(CAP_SYS_ADMIN) &&
>> + (event->attr.exclude_kernel || event->attr.exclude_hv) &&
>
> I may be missing something, but if !capable(CAP_SYS_ADMIN), should we
> still check the exclude_kernel/exclude_hv fields in the event
> attribute? Aren't those user controlled?
>
Yes that right. But i also want to handle the case when we sampling only
for userspace even with higher privilege level. May be I should handle
that as a separate patch.
I will respin this patch to check only for the privilege level and
change the commit message accordingly.
Thanks for review
Maddy
> - Naveen
>
>> + is_kernel_addr(mfspr(SPRN_SDAR)))
>> + *addrp = 0;
>> }
>>
>> static bool regs_sihv(struct pt_regs *regs)
>> @@ -2054,7 +2059,7 @@ static void record_and_restart(struct
>> perf_event *event, unsigned long val,
>>
>> if (event->attr.sample_type &
>> (PERF_SAMPLE_ADDR | PERF_SAMPLE_PHYS_ADDR))
>> - perf_get_data_addr(regs, &data.addr);
>> + perf_get_data_addr(regs, &data.addr, event);
>>
>> if (event->attr.sample_type & PERF_SAMPLE_BRANCH_STACK) {
>> struct cpu_hw_events *cpuhw;
>> --
>> 2.7.4
>>
>>
^ 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