* Re: linux-next: build warning after merge of the akpm tree
From: Stephen Rothwell @ 2020-12-09 7:07 UTC (permalink / raw)
To: Michael Ellerman
Cc: Kees Cook, Mathieu Malaterre, Linux Kernel Mailing List,
Nicholas Piggin, Linux Next Mailing List, Andrew Morton, PowerPC
In-Reply-To: <87r1nzsi4s.fsf@mpe.ellerman.id.au>
[-- Attachment #1: Type: text/plain, Size: 247 bytes --]
Hi Michael,
On Wed, 09 Dec 2020 15:44:35 +1100 Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> They should really be in DATA_DATA or similar shouldn't they?
No other architecture appears t need them ...
--
Cheers,
Stephen Rothwell
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH] powerpc/mm: Refactor the floor/ceiling check in hugetlb range freeing functions
From: Aneesh Kumar K.V @ 2020-12-09 6:29 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <16a571bb32eb6e8cd44bda484c8d81cd8a25e6d7.1604668827.git.christophe.leroy@csgroup.eu>
Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> All hugetlb range freeing functions have a verification like the following,
> which only differs by the mask used, depending on the page table level.
>
> start &= MASK;
> if (start < floor)
> return;
> if (ceiling) {
> ceiling &= MASK;
> if (! ceiling)
> return;
> }
> if (end - 1 > ceiling - 1)
> return;
>
> Refactor that into a helper function which takes the mask as
> an argument, returning true when [start;end[ is not fully
> contained inside [floor;ceiling[
>
Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> arch/powerpc/mm/hugetlbpage.c | 56 ++++++++++++-----------------------
> 1 file changed, 19 insertions(+), 37 deletions(-)
>
> diff --git a/arch/powerpc/mm/hugetlbpage.c b/arch/powerpc/mm/hugetlbpage.c
> index 36c3800769fb..f8d8a4988e15 100644
> --- a/arch/powerpc/mm/hugetlbpage.c
> +++ b/arch/powerpc/mm/hugetlbpage.c
> @@ -294,6 +294,21 @@ static void hugepd_free(struct mmu_gather *tlb, void *hugepte)
> static inline void hugepd_free(struct mmu_gather *tlb, void *hugepte) {}
> #endif
>
> +/* Return true when the entry to be freed maps more than the area being freed */
> +static bool range_is_outside_limits(unsigned long start, unsigned long end,
> + unsigned long floor, unsigned long ceiling,
> + unsigned long mask)
> +{
> + if ((start & mask) < floor)
> + return true;
> + if (ceiling) {
> + ceiling &= mask;
> + if (!ceiling)
> + return true;
> + }
> + return end - 1 > ceiling - 1;
> +}
> +
> static void free_hugepd_range(struct mmu_gather *tlb, hugepd_t *hpdp, int pdshift,
> unsigned long start, unsigned long end,
> unsigned long floor, unsigned long ceiling)
> @@ -309,15 +324,7 @@ static void free_hugepd_range(struct mmu_gather *tlb, hugepd_t *hpdp, int pdshif
> if (shift > pdshift)
> num_hugepd = 1 << (shift - pdshift);
>
> - start &= pdmask;
> - if (start < floor)
> - return;
> - if (ceiling) {
> - ceiling &= pdmask;
> - if (! ceiling)
> - return;
> - }
> - if (end - 1 > ceiling - 1)
> + if (range_is_outside_limits(start, end, floor, ceiling, pdmask))
> return;
>
> for (i = 0; i < num_hugepd; i++, hpdp++)
> @@ -334,18 +341,9 @@ static void hugetlb_free_pte_range(struct mmu_gather *tlb, pmd_t *pmd,
> unsigned long addr, unsigned long end,
> unsigned long floor, unsigned long ceiling)
> {
> - unsigned long start = addr;
> pgtable_t token = pmd_pgtable(*pmd);
>
> - start &= PMD_MASK;
> - if (start < floor)
> - return;
> - if (ceiling) {
> - ceiling &= PMD_MASK;
> - if (!ceiling)
> - return;
> - }
> - if (end - 1 > ceiling - 1)
> + if (range_is_outside_limits(addr, end, floor, ceiling, PMD_MASK))
> return;
>
> pmd_clear(pmd);
> @@ -395,15 +393,7 @@ static void hugetlb_free_pmd_range(struct mmu_gather *tlb, pud_t *pud,
> addr, next, floor, ceiling);
> } while (addr = next, addr != end);
>
> - start &= PUD_MASK;
> - if (start < floor)
> - return;
> - if (ceiling) {
> - ceiling &= PUD_MASK;
> - if (!ceiling)
> - return;
> - }
> - if (end - 1 > ceiling - 1)
> + if (range_is_outside_limits(start, end, floor, ceiling, PUD_MASK))
> return;
>
> pmd = pmd_offset(pud, start);
> @@ -446,15 +436,7 @@ static void hugetlb_free_pud_range(struct mmu_gather *tlb, p4d_t *p4d,
> }
> } while (addr = next, addr != end);
>
> - start &= PGDIR_MASK;
> - if (start < floor)
> - return;
> - if (ceiling) {
> - ceiling &= PGDIR_MASK;
> - if (!ceiling)
> - return;
> - }
> - if (end - 1 > ceiling - 1)
> + if (range_is_outside_limits(start, end, floor, ceiling, PGDIR_MASK))
> return;
>
> pud = pud_offset(p4d, start);
> --
> 2.25.0
^ permalink raw reply
* Re: [PATCH v3 4/5] powerpc/fault: Avoid heavy search_exception_tables() verification
From: Christophe Leroy @ 2020-12-09 5:34 UTC (permalink / raw)
To: Aneesh Kumar K.V, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, npiggin
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <b532a9c6-97de-031d-f880-901a117cc95c@csgroup.eu>
Le 08/12/2020 à 16:07, Christophe Leroy a écrit :
>
>
> Le 08/12/2020 à 15:52, Aneesh Kumar K.V a écrit :
>> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>>
>>> search_exception_tables() is an heavy operation, we have to avoid it.
>>> When KUAP is selected, we'll know the fault has been blocked by KUAP.
>>> Otherwise, it behaves just as if the address was already in the TLBs
>>> and no fault was generated.
>>>
>>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>>> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
>>> ---
>>> v3: rebased
>>> v2: Squashed with the preceeding patch which was re-ordering tests that get removed in this patch.
>>> ---
>>> arch/powerpc/mm/fault.c | 23 +++++++----------------
>>> 1 file changed, 7 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
>>> index 3fcd34c28e10..1770b41e4730 100644
>>> --- a/arch/powerpc/mm/fault.c
>>> +++ b/arch/powerpc/mm/fault.c
>>> @@ -210,28 +210,19 @@ static bool bad_kernel_fault(struct pt_regs *regs, unsigned long error_code,
>>> return true;
>>> }
>>> - if (!is_exec && address < TASK_SIZE && (error_code & (DSISR_PROTFAULT | DSISR_KEYFAULT)) &&
>>> - !search_exception_tables(regs->nip)) {
>>> - pr_crit_ratelimited("Kernel attempted to access user page (%lx) - exploit attempt? (uid:
>>> %d)\n",
>>> - address,
>>> - from_kuid(&init_user_ns, current_uid()));
>>> - }
>>> -
>>> // Kernel fault on kernel address is bad
>>> if (address >= TASK_SIZE)
>>> return true;
>>> - // Fault on user outside of certain regions (eg. copy_tofrom_user()) is bad
>>> - if (!search_exception_tables(regs->nip))
>>> - return true;
>>> -
>>> - // Read/write fault in a valid region (the exception table search passed
>>> - // above), but blocked by KUAP is bad, it can never succeed.
>>> - if (bad_kuap_fault(regs, address, is_write))
>>> + // Read/write fault blocked by KUAP is bad, it can never succeed.
>>> + if (bad_kuap_fault(regs, address, is_write)) {
>>> + pr_crit_ratelimited("Kernel attempted to %s user page (%lx) - exploit attempt? (uid:
>>> %d)\n",
>>> + is_write ? "write" : "read", address,
>>> + from_kuid(&init_user_ns, current_uid()));
>>> return true;
>>> + }
>>
>>
>> With this I am wondering whether the WARN() in bad_kuap_fault() is
>> needed. A direct access of userspace address will trigger this, whereas
>> previously we used bad_kuap_fault() only to identify incorrect restore
>> of AMR register (ie, to identify kernel bugs). Hence a WARN() there was
>> useful. We loose that differentiation now?
>
> Yes, I wanted to remove the WARN(), see
> https://patchwork.ozlabs.org/project/linuxppc-dev/patch/cc9129bdda1dbc2f0a09cf45fece7d0b0e690784.1605541983.git.christophe.leroy@csgroup.eu/
>
> but I understood from Michael that maybe it was not a good idea, so I left it aside for now when
> rebasing to v3.
>
> Yes previously we were able to differentiate between a direct access of userspace and a valid access
> triggering a KUAP fault, but at the cost of the heavy search_exception_tables().
> The issue was reported by Nick through https://github.com/linuxppc/issues/issues/317
>
> Should be perform the search_exception_tables() once we have hit the KUAP fault and WARN() only in
> that case ?
I sent out v4 which does that: only emit the warning once we know it is a KUAP fault within an
uaccess routine. With that, we should be back more or less as before: warning only if we hit KUAP
fault AND it is a place where a userspace access should be granted.
We are not anymore in the fast hot path, so calling search_exception_tables() there should be a
performance issue.
Christophe
>
> I was wondering also if we should keep the WARN() only when CONFIG_PPC_KUAP_DEBUG is set ?
>
^ permalink raw reply
* [PATCH v4 6/6] powerpc/fault: Perform exception fixup in do_page_fault()
From: Christophe Leroy @ 2020-12-09 5:29 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, npiggin,
aneesh.kumar
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <0d37490a067840f53fc5b118869917c0aec9ab87.1607491747.git.christophe.leroy@csgroup.eu>
Exception fixup doesn't require the heady full regs saving,
do it from do_page_fault() directly.
For that, split bad_page_fault() in two parts.
As bad_page_fault() can also be called from other places than
handle_page_fault(), it will still perform exception fixup and
fallback on __bad_page_fault().
handle_page_fault() directly calls __bad_page_fault() as the
exception fixup will now be done by do_page_fault()
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
v2: Add prototype of __bad_page_fault() in asm/bug.h
---
arch/powerpc/include/asm/bug.h | 1 +
arch/powerpc/kernel/entry_32.S | 2 +-
arch/powerpc/kernel/exceptions-64e.S | 2 +-
arch/powerpc/kernel/exceptions-64s.S | 2 +-
arch/powerpc/mm/fault.c | 33 ++++++++++++++++++++--------
5 files changed, 28 insertions(+), 12 deletions(-)
diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h
index ba0500872cce..464f8ca8a5c9 100644
--- a/arch/powerpc/include/asm/bug.h
+++ b/arch/powerpc/include/asm/bug.h
@@ -113,6 +113,7 @@
struct pt_regs;
extern int do_page_fault(struct pt_regs *, unsigned long, unsigned long);
extern void bad_page_fault(struct pt_regs *, unsigned long, int);
+void __bad_page_fault(struct pt_regs *regs, unsigned long address, int sig);
extern void _exception(int, struct pt_regs *, int, unsigned long);
extern void _exception_pkey(struct pt_regs *, unsigned long, int);
extern void die(const char *, struct pt_regs *, long);
diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
index 58177c71dfd4..1c9b0ccc2172 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -684,7 +684,7 @@ handle_page_fault:
mr r5,r3
addi r3,r1,STACK_FRAME_OVERHEAD
lwz r4,_DAR(r1)
- bl bad_page_fault
+ bl __bad_page_fault
b ret_from_except_full
#ifdef CONFIG_PPC_BOOK3S_32
diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S
index f579ce46eef2..74d07dc0bb48 100644
--- a/arch/powerpc/kernel/exceptions-64e.S
+++ b/arch/powerpc/kernel/exceptions-64e.S
@@ -1023,7 +1023,7 @@ storage_fault_common:
mr r5,r3
addi r3,r1,STACK_FRAME_OVERHEAD
ld r4,_DAR(r1)
- bl bad_page_fault
+ bl __bad_page_fault
b ret_from_except
/*
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index 1c8f1b90e174..e02ad6fefa46 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -3259,7 +3259,7 @@ handle_page_fault:
mr r5,r3
addi r3,r1,STACK_FRAME_OVERHEAD
ld r4,_DAR(r1)
- bl bad_page_fault
+ bl __bad_page_fault
b interrupt_return
/* We have a data breakpoint exception - handle it */
diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index 389a2a875262..8961b44f350c 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -545,10 +545,20 @@ NOKPROBE_SYMBOL(__do_page_fault);
int do_page_fault(struct pt_regs *regs, unsigned long address,
unsigned long error_code)
{
+ const struct exception_table_entry *entry;
enum ctx_state prev_state = exception_enter();
int rc = __do_page_fault(regs, address, error_code);
exception_exit(prev_state);
- return rc;
+ if (likely(!rc))
+ return 0;
+
+ entry = search_exception_tables(regs->nip);
+ if (unlikely(!entry))
+ return rc;
+
+ instruction_pointer_set(regs, extable_fixup(entry));
+
+ return 0;
}
NOKPROBE_SYMBOL(do_page_fault);
@@ -557,17 +567,10 @@ NOKPROBE_SYMBOL(do_page_fault);
* It is called from the DSI and ISI handlers in head.S and from some
* of the procedures in traps.c.
*/
-void bad_page_fault(struct pt_regs *regs, unsigned long address, int sig)
+void __bad_page_fault(struct pt_regs *regs, unsigned long address, int sig)
{
- const struct exception_table_entry *entry;
int is_write = page_fault_is_write(regs->dsisr);
- /* Are we prepared to handle this fault? */
- if ((entry = search_exception_tables(regs->nip)) != NULL) {
- regs->nip = extable_fixup(entry);
- return;
- }
-
/* kernel has accessed a bad area */
switch (TRAP(regs)) {
@@ -601,3 +604,15 @@ void bad_page_fault(struct pt_regs *regs, unsigned long address, int sig)
die("Kernel access of bad area", regs, sig);
}
+
+void bad_page_fault(struct pt_regs *regs, unsigned long address, int sig)
+{
+ const struct exception_table_entry *entry;
+
+ /* Are we prepared to handle this fault? */
+ entry = search_exception_tables(instruction_pointer(regs));
+ if (entry)
+ instruction_pointer_set(regs, extable_fixup(entry));
+ else
+ __bad_page_fault(regs, address, sig);
+}
--
2.25.0
^ permalink raw reply related
* [PATCH v4 4/6] powerpc/mm: Move the WARN() out of bad_kuap_fault()
From: Christophe Leroy @ 2020-12-09 5:29 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, npiggin,
aneesh.kumar
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <0d37490a067840f53fc5b118869917c0aec9ab87.1607491747.git.christophe.leroy@csgroup.eu>
In order to prepare the removal of calls to
search_exception_tables() on the fast path, move the
WARN() out of bad_kuap_fault().
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
v4: New
---
arch/powerpc/include/asm/book3s/32/kup.h | 6 +-----
arch/powerpc/include/asm/book3s/64/kup.h | 6 ++----
arch/powerpc/include/asm/nohash/32/kup-8xx.h | 3 +--
arch/powerpc/mm/fault.c | 2 +-
4 files changed, 5 insertions(+), 12 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/32/kup.h b/arch/powerpc/include/asm/book3s/32/kup.h
index 32fd4452e960..a0117a9d5b06 100644
--- a/arch/powerpc/include/asm/book3s/32/kup.h
+++ b/arch/powerpc/include/asm/book3s/32/kup.h
@@ -183,11 +183,7 @@ bad_kuap_fault(struct pt_regs *regs, unsigned long address, bool is_write)
unsigned long begin = regs->kuap & 0xf0000000;
unsigned long end = regs->kuap << 28;
- if (!is_write)
- return false;
-
- return WARN(address < begin || address >= end,
- "Bug: write fault blocked by segment registers !");
+ return is_write && (address < begin || address >= end);
}
#endif /* CONFIG_PPC_KUAP */
diff --git a/arch/powerpc/include/asm/book3s/64/kup.h b/arch/powerpc/include/asm/book3s/64/kup.h
index 7075c92c320c..f50f72e535aa 100644
--- a/arch/powerpc/include/asm/book3s/64/kup.h
+++ b/arch/powerpc/include/asm/book3s/64/kup.h
@@ -371,11 +371,9 @@ static inline bool bad_kuap_fault(struct pt_regs *regs, unsigned long address,
* the AMR. Hence check for BLOCK_WRITE/READ against AMR.
*/
if (is_write) {
- return WARN(((regs->amr & AMR_KUAP_BLOCK_WRITE) == AMR_KUAP_BLOCK_WRITE),
- "Bug: Write fault blocked by AMR!");
+ return (regs->amr & AMR_KUAP_BLOCK_WRITE) == AMR_KUAP_BLOCK_WRITE;
}
- return WARN(((regs->amr & AMR_KUAP_BLOCK_READ) == AMR_KUAP_BLOCK_READ),
- "Bug: Read fault blocked by AMR!");
+ return (regs->amr & AMR_KUAP_BLOCK_READ) == AMR_KUAP_BLOCK_READ;
}
static __always_inline void allow_user_access(void __user *to, const void __user *from,
diff --git a/arch/powerpc/include/asm/nohash/32/kup-8xx.h b/arch/powerpc/include/asm/nohash/32/kup-8xx.h
index 567cdc557402..17a4a616436f 100644
--- a/arch/powerpc/include/asm/nohash/32/kup-8xx.h
+++ b/arch/powerpc/include/asm/nohash/32/kup-8xx.h
@@ -63,8 +63,7 @@ static inline void restore_user_access(unsigned long flags)
static inline bool
bad_kuap_fault(struct pt_regs *regs, unsigned long address, bool is_write)
{
- return WARN(!((regs->kuap ^ MD_APG_KUAP) & 0xff000000),
- "Bug: fault blocked by AP register !");
+ return !((regs->kuap ^ MD_APG_KUAP) & 0xff000000);
}
#endif /* !__ASSEMBLY__ */
diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index 3fcd34c28e10..04505f938bbc 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -228,7 +228,7 @@ static bool bad_kernel_fault(struct pt_regs *regs, unsigned long error_code,
// Read/write fault in a valid region (the exception table search passed
// above), but blocked by KUAP is bad, it can never succeed.
if (bad_kuap_fault(regs, address, is_write))
- return true;
+ return WARN(true, "Bug: %s fault blocked by KUAP!", is_write ? "Write" : "Read");
// What's left? Kernel fault on user in well defined regions (extable
// matched), and allowed by KUAP in the faulting context.
--
2.25.0
^ permalink raw reply related
* [PATCH v4 3/6] powerpc/fault: Unnest definition of page_fault_is_write() and page_fault_is_bad()
From: Christophe Leroy @ 2020-12-09 5:29 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, npiggin,
aneesh.kumar
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <0d37490a067840f53fc5b118869917c0aec9ab87.1607491747.git.christophe.leroy@csgroup.eu>
To make it more readable, separate page_fault_is_write() and page_fault_is_bad()
to avoir several levels of #ifdefs
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/mm/fault.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index f6ae56a0d7a3..3fcd34c28e10 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -363,17 +363,19 @@ static void sanity_check_fault(bool is_write, bool is_user,
*/
#if (defined(CONFIG_4xx) || defined(CONFIG_BOOKE))
#define page_fault_is_write(__err) ((__err) & ESR_DST)
-#define page_fault_is_bad(__err) (0)
#else
#define page_fault_is_write(__err) ((__err) & DSISR_ISSTORE)
-#if defined(CONFIG_PPC_8xx)
+#endif
+
+#if defined(CONFIG_4xx) || defined(CONFIG_BOOKE)
+#define page_fault_is_bad(__err) (0)
+#elif defined(CONFIG_PPC_8xx)
#define page_fault_is_bad(__err) ((__err) & DSISR_NOEXEC_OR_G)
#elif defined(CONFIG_PPC64)
#define page_fault_is_bad(__err) ((__err) & DSISR_BAD_FAULT_64S)
#else
#define page_fault_is_bad(__err) ((__err) & DSISR_BAD_FAULT_32S)
#endif
-#endif
/*
* For 600- and 800-family processors, the error_code parameter is DSISR
--
2.25.0
^ permalink raw reply related
* [PATCH v4 2/6] powerpc/mm: sanity_check_fault() should work for all, not only BOOK3S
From: Christophe Leroy @ 2020-12-09 5:29 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, npiggin,
aneesh.kumar
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <0d37490a067840f53fc5b118869917c0aec9ab87.1607491747.git.christophe.leroy@csgroup.eu>
The verification and message introduced by commit 374f3f5979f9
("powerpc/mm/hash: Handle user access of kernel address gracefully")
applies to all platforms, it should not be limited to BOOK3S.
Make the BOOK3S version of sanity_check_fault() the one for all,
and bail out earlier if not BOOK3S.
Fixes: 374f3f5979f9 ("powerpc/mm/hash: Handle user access of kernel address gracefully")
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/mm/fault.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index b12595102525..f6ae56a0d7a3 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -303,7 +303,6 @@ static inline void cmo_account_page_fault(void)
static inline void cmo_account_page_fault(void) { }
#endif /* CONFIG_PPC_SMLPAR */
-#ifdef CONFIG_PPC_BOOK3S
static void sanity_check_fault(bool is_write, bool is_user,
unsigned long error_code, unsigned long address)
{
@@ -320,6 +319,9 @@ static void sanity_check_fault(bool is_write, bool is_user,
return;
}
+ if (!IS_ENABLED(CONFIG_PPC_BOOK3S))
+ return;
+
/*
* For hash translation mode, we should never get a
* PROTFAULT. Any update to pte to reduce access will result in us
@@ -354,10 +356,6 @@ static void sanity_check_fault(bool is_write, bool is_user,
WARN_ON_ONCE(error_code & DSISR_PROTFAULT);
}
-#else
-static void sanity_check_fault(bool is_write, bool is_user,
- unsigned long error_code, unsigned long address) { }
-#endif /* CONFIG_PPC_BOOK3S */
/*
* Define the correct "is_write" bit in error_code based
--
2.25.0
^ permalink raw reply related
* [PATCH v4 1/6] powerpc/book3s64/kuap: Improve error reporting with KUAP
From: Christophe Leroy @ 2020-12-09 5:29 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, npiggin,
aneesh.kumar
Cc: linuxppc-dev, linux-kernel
From: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
This partially reverts commit eb232b162446 ("powerpc/book3s64/kuap: Improve
error reporting with KUAP") and update the fault handler to print
[ 55.022514] Kernel attempted to access user page (7e6725b70000) - exploit attempt? (uid: 0)
[ 55.022528] BUG: Unable to handle kernel data access on read at 0x7e6725b70000
[ 55.022533] Faulting instruction address: 0xc000000000e8b9bc
[ 55.022540] Oops: Kernel access of bad area, sig: 11 [#1]
....
when the kernel access userspace address without unlocking AMR.
bad_kuap_fault() is added as part of commit 5e5be3aed230 ("powerpc/mm: Detect
bad KUAP faults") to catch userspace access incorrectly blocked by AMR. Hence
retain the full stack dump there even with hash translation. Also, add a comment
explaining the difference between hash and radix.
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/book3s/32/kup.h | 4 +--
arch/powerpc/include/asm/book3s/64/kup.h | 34 ++++++++++----------
arch/powerpc/include/asm/kup.h | 4 +--
arch/powerpc/include/asm/nohash/32/kup-8xx.h | 4 +--
arch/powerpc/mm/fault.c | 4 +--
5 files changed, 25 insertions(+), 25 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/32/kup.h b/arch/powerpc/include/asm/book3s/32/kup.h
index b18cd931e325..32fd4452e960 100644
--- a/arch/powerpc/include/asm/book3s/32/kup.h
+++ b/arch/powerpc/include/asm/book3s/32/kup.h
@@ -177,8 +177,8 @@ static inline void restore_user_access(unsigned long flags)
allow_user_access(to, to, end - addr, KUAP_READ_WRITE);
}
-static inline bool bad_kuap_fault(struct pt_regs *regs, unsigned long address,
- bool is_write, unsigned long error_code)
+static inline bool
+bad_kuap_fault(struct pt_regs *regs, unsigned long address, bool is_write)
{
unsigned long begin = regs->kuap & 0xf0000000;
unsigned long end = regs->kuap << 28;
diff --git a/arch/powerpc/include/asm/book3s/64/kup.h b/arch/powerpc/include/asm/book3s/64/kup.h
index f2e6dd78d5e2..7075c92c320c 100644
--- a/arch/powerpc/include/asm/book3s/64/kup.h
+++ b/arch/powerpc/include/asm/book3s/64/kup.h
@@ -353,29 +353,29 @@ static inline void set_kuap(unsigned long value)
isync();
}
-#define RADIX_KUAP_BLOCK_READ UL(0x4000000000000000)
-#define RADIX_KUAP_BLOCK_WRITE UL(0x8000000000000000)
-
static inline bool bad_kuap_fault(struct pt_regs *regs, unsigned long address,
- bool is_write, unsigned long error_code)
+ bool is_write)
{
if (!mmu_has_feature(MMU_FTR_BOOK3S_KUAP))
return false;
-
- if (radix_enabled()) {
- /*
- * Will be a storage protection fault.
- * Only check the details of AMR[0]
- */
- return WARN((regs->kuap & (is_write ? RADIX_KUAP_BLOCK_WRITE : RADIX_KUAP_BLOCK_READ)),
- "Bug: %s fault blocked by AMR!", is_write ? "Write" : "Read");
- }
/*
- * We don't want to WARN here because userspace can setup
- * keys such that a kernel access to user address can cause
- * fault
+ * For radix this will be a storage protection fault (DSISR_PROTFAULT).
+ * For hash this will be a key fault (DSISR_KEYFAULT)
*/
- return !!(error_code & DSISR_KEYFAULT);
+ /*
+ * We do have exception table entry, but accessing the
+ * userspace results in fault. This could be because we
+ * didn't unlock the AMR or access is denied by userspace
+ * using a key value that blocks access. We are only interested
+ * in catching the use case of accessing without unlocking
+ * the AMR. Hence check for BLOCK_WRITE/READ against AMR.
+ */
+ if (is_write) {
+ return WARN(((regs->amr & AMR_KUAP_BLOCK_WRITE) == AMR_KUAP_BLOCK_WRITE),
+ "Bug: Write fault blocked by AMR!");
+ }
+ return WARN(((regs->amr & AMR_KUAP_BLOCK_READ) == AMR_KUAP_BLOCK_READ),
+ "Bug: Read fault blocked by AMR!");
}
static __always_inline void allow_user_access(void __user *to, const void __user *from,
diff --git a/arch/powerpc/include/asm/kup.h b/arch/powerpc/include/asm/kup.h
index f8ec679bd2de..5a9820c54da9 100644
--- a/arch/powerpc/include/asm/kup.h
+++ b/arch/powerpc/include/asm/kup.h
@@ -62,8 +62,8 @@ void setup_kuap(bool disabled);
#else
static inline void setup_kuap(bool disabled) { }
-static inline bool bad_kuap_fault(struct pt_regs *regs, unsigned long address,
- bool is_write, unsigned long error_code)
+static inline bool
+bad_kuap_fault(struct pt_regs *regs, unsigned long address, bool is_write)
{
return false;
}
diff --git a/arch/powerpc/include/asm/nohash/32/kup-8xx.h b/arch/powerpc/include/asm/nohash/32/kup-8xx.h
index 7bdd9e5b63ed..567cdc557402 100644
--- a/arch/powerpc/include/asm/nohash/32/kup-8xx.h
+++ b/arch/powerpc/include/asm/nohash/32/kup-8xx.h
@@ -60,8 +60,8 @@ static inline void restore_user_access(unsigned long flags)
mtspr(SPRN_MD_AP, flags);
}
-static inline bool bad_kuap_fault(struct pt_regs *regs, unsigned long address,
- bool is_write, unsigned long error_code)
+static inline bool
+bad_kuap_fault(struct pt_regs *regs, unsigned long address, bool is_write)
{
return WARN(!((regs->kuap ^ MD_APG_KUAP) & 0xff000000),
"Bug: fault blocked by AP register !");
diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index c91621df0c61..b12595102525 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -210,7 +210,7 @@ static bool bad_kernel_fault(struct pt_regs *regs, unsigned long error_code,
return true;
}
- if (!is_exec && address < TASK_SIZE && (error_code & DSISR_PROTFAULT) &&
+ if (!is_exec && address < TASK_SIZE && (error_code & (DSISR_PROTFAULT | DSISR_KEYFAULT)) &&
!search_exception_tables(regs->nip)) {
pr_crit_ratelimited("Kernel attempted to access user page (%lx) - exploit attempt? (uid: %d)\n",
address,
@@ -227,7 +227,7 @@ static bool bad_kernel_fault(struct pt_regs *regs, unsigned long error_code,
// Read/write fault in a valid region (the exception table search passed
// above), but blocked by KUAP is bad, it can never succeed.
- if (bad_kuap_fault(regs, address, is_write, error_code))
+ if (bad_kuap_fault(regs, address, is_write))
return true;
// What's left? Kernel fault on user in well defined regions (extable
--
2.25.0
^ permalink raw reply related
* [PATCH v4 5/6] powerpc/fault: Avoid heavy search_exception_tables() verification
From: Christophe Leroy @ 2020-12-09 5:29 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, npiggin,
aneesh.kumar
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <0d37490a067840f53fc5b118869917c0aec9ab87.1607491747.git.christophe.leroy@csgroup.eu>
search_exception_tables() is an heavy operation, we have to avoid it.
When KUAP is selected, we'll know the fault has been blocked by KUAP.
When it is blocked by KUAP, check whether we are in an expected
userspace access place. If so, emit a warning to spot something is
going work. Otherwise, just remain silent, it will likely Oops soon.
When KUAP is not selected, it behaves just as if the address was
already in the TLBs and no fault was generated.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
---
v4: keep the search once we hit kuap_bad_fault() in order to warn or not
v3: rebased
v2: Squashed with the preceeding patch which was re-ordering tests that get removed in this patch.
---
arch/powerpc/mm/fault.c | 28 +++++++++++++---------------
1 file changed, 13 insertions(+), 15 deletions(-)
diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index 04505f938bbc..389a2a875262 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -210,28 +210,26 @@ static bool bad_kernel_fault(struct pt_regs *regs, unsigned long error_code,
return true;
}
- if (!is_exec && address < TASK_SIZE && (error_code & (DSISR_PROTFAULT | DSISR_KEYFAULT)) &&
- !search_exception_tables(regs->nip)) {
- pr_crit_ratelimited("Kernel attempted to access user page (%lx) - exploit attempt? (uid: %d)\n",
- address,
- from_kuid(&init_user_ns, current_uid()));
- }
-
// Kernel fault on kernel address is bad
if (address >= TASK_SIZE)
return true;
- // Fault on user outside of certain regions (eg. copy_tofrom_user()) is bad
- if (!search_exception_tables(regs->nip))
- return true;
+ // Read/write fault blocked by KUAP is bad, it can never succeed.
+ if (bad_kuap_fault(regs, address, is_write)) {
+ pr_crit_ratelimited("Kernel attempted to %s user page (%lx) - exploit attempt? (uid: %d)\n",
+ is_write ? "write" : "read", address,
+ from_kuid(&init_user_ns, current_uid()));
+
+ // Fault on user outside of certain regions (eg. copy_tofrom_user()) is bad
+ if (!search_exception_tables(regs->nip))
+ return true;
- // Read/write fault in a valid region (the exception table search passed
- // above), but blocked by KUAP is bad, it can never succeed.
- if (bad_kuap_fault(regs, address, is_write))
+ // Read/write fault in a valid region (the exception table search passed
+ // above), but blocked by KUAP is bad, it can never succeed.
return WARN(true, "Bug: %s fault blocked by KUAP!", is_write ? "Write" : "Read");
+ }
- // What's left? Kernel fault on user in well defined regions (extable
- // matched), and allowed by KUAP in the faulting context.
+ // What's left? Kernel fault on user and allowed by KUAP in the faulting context.
return false;
}
--
2.25.0
^ permalink raw reply related
* Re: [PATCH] powerpc/mce: Remove per cpu variables from MCE handlers
From: Michael Ellerman @ 2020-12-09 5:09 UTC (permalink / raw)
To: Mahesh Jagannath Salgaonkar, Ganesh, linuxppc-dev; +Cc: npiggin
In-Reply-To: <83ca2e1c-88f5-fc57-11e2-056f3ce835d7@linux.ibm.com>
Mahesh Jagannath Salgaonkar <mahesh@linux.ibm.com> writes:
> On 12/8/20 4:16 PM, Ganesh wrote:
>>
>> On 12/8/20 4:01 PM, Michael Ellerman wrote:
>>> Ganesh Goudar <ganeshgr@linux.ibm.com> writes:
>>>> diff --git a/arch/powerpc/include/asm/paca.h
>>>> b/arch/powerpc/include/asm/paca.h
>>>> index 9454d29ff4b4..4769954efa7d 100644
>>>> --- a/arch/powerpc/include/asm/paca.h
>>>> +++ b/arch/powerpc/include/asm/paca.h
>>>> @@ -273,6 +274,17 @@ struct paca_struct {
>>>> #ifdef CONFIG_MMIOWB
>>>> struct mmiowb_state mmiowb_state;
>>>> #endif
>>>> +#ifdef CONFIG_PPC_BOOK3S_64
>>>> + int mce_nest_count;
>>>> + struct machine_check_event mce_event[MAX_MC_EVT];
>>>> + /* Queue for delayed MCE events. */
>>>> + int mce_queue_count;
>>>> + struct machine_check_event mce_event_queue[MAX_MC_EVT];
>>>> +
>>>> + /* Queue for delayed MCE UE events. */
>>>> + int mce_ue_count;
>>>> + struct machine_check_event mce_ue_event_queue[MAX_MC_EVT];
>>>> +#endif /* CONFIG_PPC_BOOK3S_64 */
>>>> } ____cacheline_aligned;
>>> How much does this expand the paca by?
>>
>> Size of paca is 4480 bytes, these add up another 2160 bytes, so expands
>> it by 48%.
>
> Should we dynamically allocate the array sizes early as similar to that
> of paca->mce_faulty_slbs so that we don't bump up paca size ?
Yeah I think that would be preferable.
That way those allocations can be normal node-local allocations on bare
metal, or when using radix. (Or even on KVM).
In fact what we probably want is a separate struct for all the MCE
related data, eg something like:
struct mce_stuff {
int nest_count;
/* Queue for delayed MCE events. */
int queue_count;
/* Queue for delayed MCE UE events. */
int mce_ue_count;
struct machine_check_event events[MAX_MC_EVT];
struct machine_check_event event_queue[MAX_MC_EVT];
struct machine_check_event ue_event_queue[MAX_MC_EVT];
};
And then you allocate one of those per CPU, inside the RMO for pseries
with hash, and node-local otherwise.
cheers
^ permalink raw reply
* Re: linux-next: build warning after merge of the akpm tree
From: Michael Ellerman @ 2020-12-09 4:44 UTC (permalink / raw)
To: Stephen Rothwell, Andrew Morton
Cc: Kees Cook, Mathieu Malaterre, Linux Kernel Mailing List,
Nicholas Piggin, Linux Next Mailing List, PowerPC
In-Reply-To: <20201208230157.42c42789@canb.auug.org.au>
Stephen Rothwell <sfr@canb.auug.org.au> writes:
> Hi Stephen,
>
> On Fri, 4 Dec 2020 21:00:00 +1100 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>>
>> Hi all,
>>
>> After merging the akpm tree, today's linux-next build (powerpc
>> allyesconfig) produced warnings like this:
>>
>> ld: warning: orphan section `.data..Lubsan_data177' from `arch/powerpc/oprofile/op_model_pa6t.o' being placed in section `.data..Lubsan_data177'
>>
>> (lots of these latter ones)
>
> 781584 of them today!
>
>> I don't know what produced these, but it is in the akpm-current or
>> akpm trees.
>
> Presumably the result of commit
>
> 186c3e18dba3 ("ubsan: enable for all*config builds")
>
> from the akpm-current tree.
>
> arch/powerpc/kernel/vmlinux.lds.S has:
>
> #ifdef CONFIG_PPC32
> .data : AT(ADDR(.data) - LOAD_OFFSET) {
> DATA_DATA
> #ifdef CONFIG_UBSAN
> *(.data..Lubsan_data*)
> *(.data..Lubsan_type*)
> #endif
> *(.data.rel*)
> *(SDATA_MAIN)
>
> added by commit
>
> beba24ac5913 ("powerpc/32: Add .data..Lubsan_data*/.data..Lubsan_type* sections explicitly")
>
> in 2018, but no equivalent for 64 bit.
They should really be in DATA_DATA or similar shouldn't they?
cheers
^ permalink raw reply
* [powerpc:next-test] BUILD SUCCESS db972a3787d12b1ce9ba7a31ec376d8a79e04c47
From: kernel test robot @ 2020-12-09 4:42 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next-test
branch HEAD: db972a3787d12b1ce9ba7a31ec376d8a79e04c47 powerpc/powermac: Fix low_sleep_handler with CONFIG_VMAP_STACK
elapsed time: 884m
configs tested: 103
configs skipped: 3
The following configs have been built successfully.
More configs may be tested in the coming days.
gcc tested configs:
arm defconfig
arm64 allyesconfig
arm64 defconfig
arm allmodconfig
arm allyesconfig
arm lubbock_defconfig
sh apsh4ad0a_defconfig
m68k sun3_defconfig
sh migor_defconfig
powerpc g5_defconfig
powerpc skiroot_defconfig
sh rts7751r2dplus_defconfig
sh magicpanelr2_defconfig
sh rsk7264_defconfig
arc haps_hs_smp_defconfig
sh se7724_defconfig
powerpc ep8248e_defconfig
arm assabet_defconfig
mips omega2p_defconfig
arm dove_defconfig
mips ath79_defconfig
powerpc kmeter1_defconfig
mips maltaaprp_defconfig
powerpc mgcoge_defconfig
arc hsdk_defconfig
xtensa defconfig
powerpc pmac32_defconfig
sh se7721_defconfig
c6x evmc6457_defconfig
powerpc acadia_defconfig
mips bcm47xx_defconfig
arm tango4_defconfig
powerpc mpc7448_hpc2_defconfig
ia64 allmodconfig
ia64 defconfig
ia64 allyesconfig
m68k allmodconfig
m68k defconfig
m68k allyesconfig
nios2 defconfig
arc allyesconfig
nds32 allnoconfig
c6x allyesconfig
nds32 defconfig
nios2 allyesconfig
csky defconfig
alpha defconfig
alpha allyesconfig
xtensa allyesconfig
h8300 allyesconfig
arc defconfig
sh allmodconfig
parisc defconfig
s390 allyesconfig
parisc allyesconfig
s390 defconfig
i386 allyesconfig
sparc allyesconfig
sparc defconfig
i386 tinyconfig
i386 defconfig
mips allyesconfig
mips allmodconfig
powerpc allyesconfig
powerpc allmodconfig
powerpc allnoconfig
x86_64 randconfig-a004-20201208
x86_64 randconfig-a006-20201208
x86_64 randconfig-a005-20201208
x86_64 randconfig-a001-20201208
x86_64 randconfig-a002-20201208
x86_64 randconfig-a003-20201208
i386 randconfig-a004-20201208
i386 randconfig-a005-20201208
i386 randconfig-a001-20201208
i386 randconfig-a002-20201208
i386 randconfig-a006-20201208
i386 randconfig-a003-20201208
i386 randconfig-a013-20201208
i386 randconfig-a014-20201208
i386 randconfig-a011-20201208
i386 randconfig-a015-20201208
i386 randconfig-a012-20201208
i386 randconfig-a016-20201208
riscv nommu_k210_defconfig
riscv allyesconfig
riscv nommu_virt_defconfig
riscv allnoconfig
riscv defconfig
riscv rv32_defconfig
riscv allmodconfig
x86_64 rhel
x86_64 allyesconfig
x86_64 rhel-7.6-kselftests
x86_64 defconfig
x86_64 rhel-8.3
x86_64 kexec
clang tested configs:
x86_64 randconfig-a016-20201208
x86_64 randconfig-a012-20201208
x86_64 randconfig-a013-20201208
x86_64 randconfig-a014-20201208
x86_64 randconfig-a015-20201208
x86_64 randconfig-a011-20201208
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* [powerpc:fixes-test] BUILD SUCCESS 5eedf9fe8db23313df104576845cec5f481b9b60
From: kernel test robot @ 2020-12-09 4:42 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git fixes-test
branch HEAD: 5eedf9fe8db23313df104576845cec5f481b9b60 powerpc/mm: Fix KUAP warning by providing copy_from_kernel_nofault_allowed()
elapsed time: 884m
configs tested: 129
configs skipped: 2
The following configs have been built successfully.
More configs may be tested in the coming days.
gcc tested configs:
arm64 allyesconfig
arm64 defconfig
arm allyesconfig
arm allmodconfig
arm defconfig
powerpc g5_defconfig
powerpc skiroot_defconfig
sh rts7751r2dplus_defconfig
sh magicpanelr2_defconfig
powerpc maple_defconfig
arm rpc_defconfig
parisc generic-32bit_defconfig
arm milbeaut_m10v_defconfig
powerpc warp_defconfig
sh lboxre2_defconfig
powerpc64 defconfig
powerpc pseries_defconfig
powerpc canyonlands_defconfig
powerpc mpc834x_mds_defconfig
sh apsh4a3a_defconfig
x86_64 alldefconfig
powerpc wii_defconfig
powerpc mpc83xx_defconfig
sh rsk7264_defconfig
arc haps_hs_smp_defconfig
sh se7724_defconfig
powerpc ep8248e_defconfig
arm assabet_defconfig
arm corgi_defconfig
sh alldefconfig
powerpc mpc8272_ads_defconfig
sh sdk7780_defconfig
sh se7619_defconfig
mips cu1830-neo_defconfig
sh se7751_defconfig
arm lart_defconfig
powerpc mpc7448_hpc2_defconfig
sh se7721_defconfig
mips omega2p_defconfig
arm dove_defconfig
mips ath79_defconfig
powerpc kmeter1_defconfig
mips maltaaprp_defconfig
powerpc mgcoge_defconfig
arc hsdk_defconfig
xtensa defconfig
powerpc pmac32_defconfig
xtensa virt_defconfig
m68k m5475evb_defconfig
mips mpc30x_defconfig
h8300 defconfig
openrisc simple_smp_defconfig
mips rt305x_defconfig
powerpc tqm8541_defconfig
c6x evmc6678_defconfig
powerpc tqm5200_defconfig
powerpc acadia_defconfig
arc nsimosci_defconfig
arm omap1_defconfig
ia64 allmodconfig
ia64 defconfig
ia64 allyesconfig
m68k allmodconfig
m68k defconfig
m68k allyesconfig
nios2 defconfig
arc allyesconfig
nds32 allnoconfig
c6x allyesconfig
nds32 defconfig
nios2 allyesconfig
csky defconfig
alpha defconfig
alpha allyesconfig
xtensa allyesconfig
h8300 allyesconfig
arc defconfig
sh allmodconfig
parisc defconfig
s390 allyesconfig
parisc allyesconfig
s390 defconfig
i386 allyesconfig
sparc allyesconfig
sparc defconfig
i386 tinyconfig
i386 defconfig
mips allyesconfig
mips allmodconfig
powerpc allyesconfig
powerpc allmodconfig
powerpc allnoconfig
i386 randconfig-a004-20201208
i386 randconfig-a005-20201208
i386 randconfig-a001-20201208
i386 randconfig-a002-20201208
i386 randconfig-a006-20201208
i386 randconfig-a003-20201208
x86_64 randconfig-a004-20201208
x86_64 randconfig-a006-20201208
x86_64 randconfig-a005-20201208
x86_64 randconfig-a001-20201208
x86_64 randconfig-a002-20201208
x86_64 randconfig-a003-20201208
i386 randconfig-a013-20201208
i386 randconfig-a014-20201208
i386 randconfig-a011-20201208
i386 randconfig-a015-20201208
i386 randconfig-a012-20201208
i386 randconfig-a016-20201208
riscv allyesconfig
riscv allnoconfig
riscv defconfig
riscv allmodconfig
riscv nommu_k210_defconfig
riscv nommu_virt_defconfig
riscv rv32_defconfig
x86_64 rhel
x86_64 allyesconfig
x86_64 rhel-7.6-kselftests
x86_64 defconfig
x86_64 rhel-8.3
x86_64 kexec
clang tested configs:
x86_64 randconfig-a012-20201208
x86_64 randconfig-a013-20201208
x86_64 randconfig-a011-20201208
x86_64 randconfig-a016-20201208
x86_64 randconfig-a014-20201208
x86_64 randconfig-a015-20201208
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* Re: [PATCH 04/13] powerpc/xive: Use cpu_to_node() instead of ibm, chip-id property
From: Aneesh Kumar K.V @ 2020-12-09 4:39 UTC (permalink / raw)
To: Cédric Le Goater, linuxppc-dev; +Cc: Cédric Le Goater
In-Reply-To: <20201208151124.1329942-5-clg@kaod.org>
Cédric Le Goater <clg@kaod.org> writes:
> The 'chip_id' field of the XIVE CPU structure is used to choose a
> target for a source located on the same chip when possible. This field
> is assigned on the PowerNV platform using the "ibm,chip-id" property
> on pSeries under KVM when NUMA nodes are defined but it is undefined
> under PowerVM. The XIVE source structure has a similar field
> 'src_chip' which is only assigned on the PowerNV platform.
>
> cpu_to_node() returns a compatible value on all platforms, 0 being the
> default node. It will also give us the opportunity to set the affinity
> of a source on pSeries when we can localize them.
But we should avoid assuming that linux numa node number is equivalent
to chip id [1]. What do we expect this value represents on virtualized
platforms like PowerVM and KVM? Is this used for a hcall?
[1] https://lore.kernel.org/linuxppc-dev/20200817103238.158133-1-aneesh.kumar@linux.ibm.com
>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> arch/powerpc/sysdev/xive/common.c | 7 +------
> 1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/arch/powerpc/sysdev/xive/common.c b/arch/powerpc/sysdev/xive/common.c
> index ee375daf8114..605238ca65e4 100644
> --- a/arch/powerpc/sysdev/xive/common.c
> +++ b/arch/powerpc/sysdev/xive/common.c
> @@ -1342,16 +1342,11 @@ static int xive_prepare_cpu(unsigned int cpu)
>
> xc = per_cpu(xive_cpu, cpu);
> if (!xc) {
> - struct device_node *np;
> -
> xc = kzalloc_node(sizeof(struct xive_cpu),
> GFP_KERNEL, cpu_to_node(cpu));
> if (!xc)
> return -ENOMEM;
> - np = of_get_cpu_node(cpu, NULL);
> - if (np)
> - xc->chip_id = of_get_ibm_chip_id(np);
> - of_node_put(np);
> + xc->chip_id = cpu_to_node(cpu);
> xc->hw_ipi = XIVE_BAD_IRQ;
>
> per_cpu(xive_cpu, cpu) = xc;
> --
> 2.26.2
^ permalink raw reply
* Re: [PATCH v2 4/4] KVM: PPC: Introduce new capability for 2nd DAWR
From: Paul Mackerras @ 2020-12-09 4:36 UTC (permalink / raw)
To: Ravi Bangoria
Cc: christophe.leroy, leobras.c, mikey, kvm, linux-kernel, npiggin,
kvm-ppc, jniethe5, pbonzini, linuxppc-dev
In-Reply-To: <20201124105953.39325-5-ravi.bangoria@linux.ibm.com>
On Tue, Nov 24, 2020 at 04:29:53PM +0530, Ravi Bangoria wrote:
> Introduce KVM_CAP_PPC_DAWR1 which can be used by Qemu to query whether
> kvm supports 2nd DAWR or not.
This should be described in Documentation/virt/kvm/api.rst.
Strictly speaking, it should be a capability which is disabled by
default, so the guest can only do the H_SET_MODE to set DAWR[X]1 if it
has been explicitly permitted to do so by userspace (QEMU). This is
because we want as little as possible of the VM configuration to come
from the host capabilities rather than from what userspace configures.
So what we really need here is for this to be a capability which can
be queried by userspace to find out if it is possible, and then
enabled by userspace if it wants. See how KVM_CAP_PPC_NESTED_HV is
handled for example.
Paul.
^ permalink raw reply
* Re: [PATCH v1 1/2] KVM: PPC: Book3S HV: Add support for H_RPT_INVALIDATE (nested case only)
From: Paul Mackerras @ 2020-12-09 4:15 UTC (permalink / raw)
To: Bharata B Rao; +Cc: aneesh.kumar, npiggin, kvm-ppc, linuxppc-dev, David Gibson
In-Reply-To: <20201019112642.53016-2-bharata@linux.ibm.com>
On Mon, Oct 19, 2020 at 04:56:41PM +0530, Bharata B Rao wrote:
> Implements H_RPT_INVALIDATE hcall and supports only nested case
> currently.
>
> A KVM capability KVM_CAP_RPT_INVALIDATE is added to indicate the
> support for this hcall.
I have a couple of questions about this patch:
1. Is this something that is useful today, or is it something that may
become useful in the future depending on future product plans? In
other words, what advantage is there to forcing L2 guests to use this
hcall instead of doing tlbie themselves?
2. Why does it need to be added to the default-enabled hcall list?
There is a concern that if this is enabled by default we could get the
situation where a guest using it gets migrated to a host that doesn't
support it, which would be bad. That is the reason that all new
things like this are disabled by default and only enabled by userspace
(i.e. QEMU) in situations where we can enforce that it is available on
all hosts to which the VM might be migrated.
Thanks,
Paul.
^ permalink raw reply
* Re: [PATCH 1/3] powerpc/smp: Parse ibm, thread-groups with multiple properties
From: Michael Ellerman @ 2020-12-09 3:59 UTC (permalink / raw)
To: Gautham R Shenoy, Srikar Dronamraju
Cc: Nathan Lynch, Gautham R. Shenoy, Michael Neuling,
Vaidyanathan Srinivasan, Peter Zijlstra, linux-kernel,
Nicholas Piggin, linuxppc-dev, Valentin Schneider
In-Reply-To: <20201208172540.GA14206@in.ibm.com>
Gautham R Shenoy <ego@linux.vnet.ibm.com> writes:
> Hello Srikar,
>
> Thanks for taking a look at the patch.
>
> On Mon, Dec 07, 2020 at 05:40:42PM +0530, Srikar Dronamraju wrote:
>> * Gautham R. Shenoy <ego@linux.vnet.ibm.com> [2020-12-04 10:18:45]:
>>
>> > From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
>>
>> <snipped>
>>
>> >
>> > static int parse_thread_groups(struct device_node *dn,
>> > - struct thread_groups *tg,
>> > - unsigned int property)
>> > + struct thread_groups_list *tglp)
>> > {
>> > - int i;
>> > - u32 thread_group_array[3 + MAX_THREAD_LIST_SIZE];
>> > + int i = 0;
>> > + u32 *thread_group_array;
>> > u32 *thread_list;
>> > size_t total_threads;
>> > - int ret;
>> > + int ret = 0, count;
>> > + unsigned int property_idx = 0;
>>
>> NIT:
>> tglx mentions in one of his recent comments to try keep a reverse fir tree
>> ordering of variables where possible.
>
> I suppose you mean moving the longer local variable declarations to to
> the top and shorter ones to the bottom. Thanks. Will fix this.
Yeah. It's called "reverse christmas tree", that's googleable.
I also prefer that style, it makes the locals visually sit with the
beginning of the function body.
cheers
^ permalink raw reply
* Re: [PATCH] drivers: usb: gadget: prefer pr_*() functions over raw printk()
From: Peter Chen @ 2020-12-09 1:48 UTC (permalink / raw)
To: Enrico Weigelt, metux IT consult
Cc: balbi@kernel.org, linux-usb@vger.kernel.org,
linux-kernel@vger.kernel.org, Leo Li,
laurent.pinchart@ideasonboard.com, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20201208144403.22097-1-info@metux.net>
On 20-12-08 15:44:03, Enrico Weigelt, metux IT consult wrote:
> Reduce a bit logging boilerplate by using the preferred pr_*()
> macros instead of raw printk().
It is the device driver code, it is better to use dev_info/dev_dbg.
Peter
>
> Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
> ---
> drivers/usb/gadget/function/uvc.h | 2 +-
> drivers/usb/gadget/udc/atmel_usba_udc.c | 2 +-
> drivers/usb/gadget/udc/fsl_udc_core.c | 4 +--
> drivers/usb/gadget/udc/fsl_usb2_udc.h | 4 +--
> drivers/usb/gadget/udc/fusb300_udc.c | 64 ++++++++++++++++-----------------
> drivers/usb/gadget/udc/goku_udc.c | 2 +-
> drivers/usb/gadget/udc/r8a66597-udc.h | 2 +-
> 7 files changed, 40 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/uvc.h b/drivers/usb/gadget/function/uvc.h
> index 23ee25383c1f..d546eb7c348c 100644
> --- a/drivers/usb/gadget/function/uvc.h
> +++ b/drivers/usb/gadget/function/uvc.h
> @@ -49,7 +49,7 @@ extern unsigned int uvc_gadget_trace_param;
> #define uvc_trace(flag, msg...) \
> do { \
> if (uvc_gadget_trace_param & flag) \
> - printk(KERN_DEBUG "uvcvideo: " msg); \
> + pr_debug("uvcvideo: " msg); \
> } while (0)
>
> #define uvcg_dbg(f, fmt, args...) \
> diff --git a/drivers/usb/gadget/udc/atmel_usba_udc.c b/drivers/usb/gadget/udc/atmel_usba_udc.c
> index 2b893bceea45..4834fafb3f70 100644
> --- a/drivers/usb/gadget/udc/atmel_usba_udc.c
> +++ b/drivers/usb/gadget/udc/atmel_usba_udc.c
> @@ -1573,7 +1573,7 @@ static void usba_control_irq(struct usba_udc *udc, struct usba_ep *ep)
> * generate or receive a reply right away. */
> usba_ep_writel(ep, CLR_STA, USBA_RX_SETUP);
>
> - /* printk(KERN_DEBUG "setup: %d: %02x.%02x\n",
> + /* pr_debug("setup: %d: %02x.%02x\n",
> ep->state, crq.crq.bRequestType,
> crq.crq.bRequest); */
>
> diff --git a/drivers/usb/gadget/udc/fsl_udc_core.c b/drivers/usb/gadget/udc/fsl_udc_core.c
> index ad6ff9c4188e..cab4def04f9f 100644
> --- a/drivers/usb/gadget/udc/fsl_udc_core.c
> +++ b/drivers/usb/gadget/udc/fsl_udc_core.c
> @@ -1474,7 +1474,7 @@ __acquires(udc->lock)
> mdelay(10);
> tmp = fsl_readl(&dr_regs->portsc1) | (ptc << 16);
> fsl_writel(tmp, &dr_regs->portsc1);
> - printk(KERN_INFO "udc: switch to test mode %d.\n", ptc);
> + pr_info("udc: switch to test mode %d.\n", ptc);
> }
>
> return;
> @@ -1952,7 +1952,7 @@ static int fsl_udc_start(struct usb_gadget *g,
> if (!IS_ERR_OR_NULL(udc_controller->transceiver)) {
> /* Suspend the controller until OTG enable it */
> udc_controller->stopped = 1;
> - printk(KERN_INFO "Suspend udc for OTG auto detect\n");
> + pr_info("Suspend udc for OTG auto detect\n");
>
> /* connect to bus through transceiver */
> if (!IS_ERR_OR_NULL(udc_controller->transceiver)) {
> diff --git a/drivers/usb/gadget/udc/fsl_usb2_udc.h b/drivers/usb/gadget/udc/fsl_usb2_udc.h
> index 4ba651ae9048..b180bf14dd0c 100644
> --- a/drivers/usb/gadget/udc/fsl_usb2_udc.h
> +++ b/drivers/usb/gadget/udc/fsl_usb2_udc.h
> @@ -509,7 +509,7 @@ struct fsl_udc {
> /*-------------------------------------------------------------------------*/
>
> #ifdef DEBUG
> -#define DBG(fmt, args...) printk(KERN_DEBUG "[%s] " fmt "\n", \
> +#define DBG(fmt, args...) pr_debug("[%s] " fmt "\n", \
> __func__, ## args)
> #else
> #define DBG(fmt, args...) do{}while(0)
> @@ -535,7 +535,7 @@ static void dump_msg(const char *label, const u8 * buf, unsigned int length)
> p += 3;
> }
> *p = 0;
> - printk(KERN_DEBUG "%6x: %s\n", start, line);
> + pr_debug("%6x: %s\n", start, line);
> buf += num;
> start += num;
> length -= num;
> diff --git a/drivers/usb/gadget/udc/fusb300_udc.c b/drivers/usb/gadget/udc/fusb300_udc.c
> index 9af8b415f303..c4e7e4b8e46f 100644
> --- a/drivers/usb/gadget/udc/fusb300_udc.c
> +++ b/drivers/usb/gadget/udc/fusb300_udc.c
> @@ -352,24 +352,24 @@ static void fusb300_wrcxf(struct fusb300_ep *ep,
> for (i = length >> 2; i > 0; i--) {
> data = *tmp | *(tmp + 1) << 8 | *(tmp + 2) << 16 |
> *(tmp + 3) << 24;
> - printk(KERN_DEBUG " 0x%x\n", data);
> + pr_debug(" 0x%x\n", data);
> iowrite32(data, fusb300->reg + FUSB300_OFFSET_CXPORT);
> tmp = tmp + 4;
> }
> switch (length % 4) {
> case 1:
> data = *tmp;
> - printk(KERN_DEBUG " 0x%x\n", data);
> + pr_debug(" 0x%x\n", data);
> iowrite32(data, fusb300->reg + FUSB300_OFFSET_CXPORT);
> break;
> case 2:
> data = *tmp | *(tmp + 1) << 8;
> - printk(KERN_DEBUG " 0x%x\n", data);
> + pr_debug(" 0x%x\n", data);
> iowrite32(data, fusb300->reg + FUSB300_OFFSET_CXPORT);
> break;
> case 3:
> data = *tmp | *(tmp + 1) << 8 | *(tmp + 2) << 16;
> - printk(KERN_DEBUG " 0x%x\n", data);
> + pr_debug(" 0x%x\n", data);
> iowrite32(data, fusb300->reg + FUSB300_OFFSET_CXPORT);
> break;
> default:
> @@ -390,7 +390,7 @@ static void fusb300_clear_epnstall(struct fusb300 *fusb300, u8 ep)
> u32 reg = ioread32(fusb300->reg + FUSB300_OFFSET_EPSET0(ep));
>
> if (reg & FUSB300_EPSET0_STL) {
> - printk(KERN_DEBUG "EP%d stall... Clear!!\n", ep);
> + pr_debug("EP%d stall... Clear!!\n", ep);
> reg |= FUSB300_EPSET0_STL_CLR;
> iowrite32(reg, fusb300->reg + FUSB300_OFFSET_EPSET0(ep));
> }
> @@ -402,7 +402,7 @@ static void ep0_queue(struct fusb300_ep *ep, struct fusb300_request *req)
> if (req->req.length) {
> fusb300_wrcxf(ep, req);
> } else
> - printk(KERN_DEBUG "%s : req->req.length = 0x%x\n",
> + pr_debug("%s : req->req.length = 0x%x\n",
> __func__, req->req.length);
> if ((req->req.length == req->req.actual) ||
> (req->req.actual < ep->ep.maxpacket))
> @@ -565,7 +565,7 @@ static void fusb300_rdcxf(struct fusb300 *fusb300,
>
> for (i = (length >> 2); i > 0; i--) {
> data = ioread32(fusb300->reg + FUSB300_OFFSET_CXPORT);
> - printk(KERN_DEBUG " 0x%x\n", data);
> + pr_debug(" 0x%x\n", data);
> *tmp = data & 0xFF;
> *(tmp + 1) = (data >> 8) & 0xFF;
> *(tmp + 2) = (data >> 16) & 0xFF;
> @@ -576,18 +576,18 @@ static void fusb300_rdcxf(struct fusb300 *fusb300,
> switch (length % 4) {
> case 1:
> data = ioread32(fusb300->reg + FUSB300_OFFSET_CXPORT);
> - printk(KERN_DEBUG " 0x%x\n", data);
> + pr_debug(" 0x%x\n", data);
> *tmp = data & 0xFF;
> break;
> case 2:
> data = ioread32(fusb300->reg + FUSB300_OFFSET_CXPORT);
> - printk(KERN_DEBUG " 0x%x\n", data);
> + pr_debug(" 0x%x\n", data);
> *tmp = data & 0xFF;
> *(tmp + 1) = (data >> 8) & 0xFF;
> break;
> case 3:
> data = ioread32(fusb300->reg + FUSB300_OFFSET_CXPORT);
> - printk(KERN_DEBUG " 0x%x\n", data);
> + pr_debug(" 0x%x\n", data);
> *tmp = data & 0xFF;
> *(tmp + 1) = (data >> 8) & 0xFF;
> *(tmp + 2) = (data >> 16) & 0xFF;
> @@ -610,7 +610,7 @@ static void fusb300_rdfifo(struct fusb300_ep *ep,
> req->req.actual += length;
>
> if (req->req.actual > req->req.length)
> - printk(KERN_DEBUG "req->req.actual > req->req.length\n");
> + pr_debug("req->req.actual > req->req.length\n");
>
> for (i = (length >> 2); i > 0; i--) {
> data = ioread32(fusb300->reg +
> @@ -649,7 +649,7 @@ static void fusb300_rdfifo(struct fusb300_ep *ep,
> reg = ioread32(fusb300->reg + FUSB300_OFFSET_IGR1);
> reg &= FUSB300_IGR1_SYNF0_EMPTY_INT;
> if (i)
> - printk(KERN_INFO "sync fifo is not empty!\n");
> + pr_info("sync fifo is not empty!\n");
> i++;
> } while (!reg);
> }
> @@ -677,7 +677,7 @@ static u8 fusb300_get_cxstall(struct fusb300 *fusb300)
> static void request_error(struct fusb300 *fusb300)
> {
> fusb300_set_cxstall(fusb300);
> - printk(KERN_DEBUG "request error!!\n");
> + pr_debug("request error!!\n");
> }
>
> static void get_status(struct fusb300 *fusb300, struct usb_ctrlrequest *ctrl)
> @@ -999,7 +999,7 @@ static void check_device_mode(struct fusb300 *fusb300)
> fusb300->gadget.speed = USB_SPEED_UNKNOWN;
> break;
> }
> - printk(KERN_INFO "dev_mode = %d\n", (reg & FUSB300_GCR_DEVEN_MSK));
> + pr_info("dev_mode = %d\n", (reg & FUSB300_GCR_DEVEN_MSK));
> }
>
>
> @@ -1076,14 +1076,14 @@ static irqreturn_t fusb300_irq(int irq, void *_fusb300)
> if (int_grp1 & FUSB300_IGR1_WARM_RST_INT) {
> fusb300_clear_int(fusb300, FUSB300_OFFSET_IGR1,
> FUSB300_IGR1_WARM_RST_INT);
> - printk(KERN_INFO"fusb300_warmreset\n");
> + pr_info("fusb300_warmreset\n");
> fusb300_reset();
> }
>
> if (int_grp1 & FUSB300_IGR1_HOT_RST_INT) {
> fusb300_clear_int(fusb300, FUSB300_OFFSET_IGR1,
> FUSB300_IGR1_HOT_RST_INT);
> - printk(KERN_INFO"fusb300_hotreset\n");
> + pr_info("fusb300_hotreset\n");
> fusb300_reset();
> }
>
> @@ -1097,13 +1097,13 @@ static irqreturn_t fusb300_irq(int irq, void *_fusb300)
> if (int_grp1 & FUSB300_IGR1_CX_COMABT_INT) {
> fusb300_clear_int(fusb300, FUSB300_OFFSET_IGR1,
> FUSB300_IGR1_CX_COMABT_INT);
> - printk(KERN_INFO"fusb300_ep0abt\n");
> + pr_info("fusb300_ep0abt\n");
> }
>
> if (int_grp1 & FUSB300_IGR1_VBUS_CHG_INT) {
> fusb300_clear_int(fusb300, FUSB300_OFFSET_IGR1,
> FUSB300_IGR1_VBUS_CHG_INT);
> - printk(KERN_INFO"fusb300_vbus_change\n");
> + pr_info("fusb300_vbus_change\n");
> }
>
> if (int_grp1 & FUSB300_IGR1_U3_EXIT_FAIL_INT) {
> @@ -1134,25 +1134,25 @@ static irqreturn_t fusb300_irq(int irq, void *_fusb300)
> if (int_grp1 & FUSB300_IGR1_U3_EXIT_INT) {
> fusb300_clear_int(fusb300, FUSB300_OFFSET_IGR1,
> FUSB300_IGR1_U3_EXIT_INT);
> - printk(KERN_INFO "FUSB300_IGR1_U3_EXIT_INT\n");
> + pr_info("FUSB300_IGR1_U3_EXIT_INT\n");
> }
>
> if (int_grp1 & FUSB300_IGR1_U2_EXIT_INT) {
> fusb300_clear_int(fusb300, FUSB300_OFFSET_IGR1,
> FUSB300_IGR1_U2_EXIT_INT);
> - printk(KERN_INFO "FUSB300_IGR1_U2_EXIT_INT\n");
> + pr_info("FUSB300_IGR1_U2_EXIT_INT\n");
> }
>
> if (int_grp1 & FUSB300_IGR1_U1_EXIT_INT) {
> fusb300_clear_int(fusb300, FUSB300_OFFSET_IGR1,
> FUSB300_IGR1_U1_EXIT_INT);
> - printk(KERN_INFO "FUSB300_IGR1_U1_EXIT_INT\n");
> + pr_info("FUSB300_IGR1_U1_EXIT_INT\n");
> }
>
> if (int_grp1 & FUSB300_IGR1_U3_ENTRY_INT) {
> fusb300_clear_int(fusb300, FUSB300_OFFSET_IGR1,
> FUSB300_IGR1_U3_ENTRY_INT);
> - printk(KERN_INFO "FUSB300_IGR1_U3_ENTRY_INT\n");
> + pr_info("FUSB300_IGR1_U3_ENTRY_INT\n");
> fusb300_enable_bit(fusb300, FUSB300_OFFSET_SSCR1,
> FUSB300_SSCR1_GO_U3_DONE);
> }
> @@ -1160,31 +1160,31 @@ static irqreturn_t fusb300_irq(int irq, void *_fusb300)
> if (int_grp1 & FUSB300_IGR1_U2_ENTRY_INT) {
> fusb300_clear_int(fusb300, FUSB300_OFFSET_IGR1,
> FUSB300_IGR1_U2_ENTRY_INT);
> - printk(KERN_INFO "FUSB300_IGR1_U2_ENTRY_INT\n");
> + pr_info("FUSB300_IGR1_U2_ENTRY_INT\n");
> }
>
> if (int_grp1 & FUSB300_IGR1_U1_ENTRY_INT) {
> fusb300_clear_int(fusb300, FUSB300_OFFSET_IGR1,
> FUSB300_IGR1_U1_ENTRY_INT);
> - printk(KERN_INFO "FUSB300_IGR1_U1_ENTRY_INT\n");
> + pr_info("FUSB300_IGR1_U1_ENTRY_INT\n");
> }
>
> if (int_grp1 & FUSB300_IGR1_RESM_INT) {
> fusb300_clear_int(fusb300, FUSB300_OFFSET_IGR1,
> FUSB300_IGR1_RESM_INT);
> - printk(KERN_INFO "fusb300_resume\n");
> + pr_info("fusb300_resume\n");
> }
>
> if (int_grp1 & FUSB300_IGR1_SUSP_INT) {
> fusb300_clear_int(fusb300, FUSB300_OFFSET_IGR1,
> FUSB300_IGR1_SUSP_INT);
> - printk(KERN_INFO "fusb300_suspend\n");
> + pr_info("fusb300_suspend\n");
> }
>
> if (int_grp1 & FUSB300_IGR1_HS_LPM_INT) {
> fusb300_clear_int(fusb300, FUSB300_OFFSET_IGR1,
> FUSB300_IGR1_HS_LPM_INT);
> - printk(KERN_INFO "fusb300_HS_LPM_INT\n");
> + pr_info("fusb300_HS_LPM_INT\n");
> }
>
> if (int_grp1 & FUSB300_IGR1_DEV_MODE_CHG_INT) {
> @@ -1195,11 +1195,11 @@ static irqreturn_t fusb300_irq(int irq, void *_fusb300)
>
> if (int_grp1 & FUSB300_IGR1_CX_COMFAIL_INT) {
> fusb300_set_cxstall(fusb300);
> - printk(KERN_INFO "fusb300_ep0fail\n");
> + pr_info("fusb300_ep0fail\n");
> }
>
> if (int_grp1 & FUSB300_IGR1_CX_SETUP_INT) {
> - printk(KERN_INFO "fusb300_ep0setup\n");
> + pr_info("fusb300_ep0setup\n");
> if (setup_packet(fusb300, &ctrl)) {
> spin_unlock(&fusb300->lock);
> if (fusb300->driver->setup(&fusb300->gadget, &ctrl) < 0)
> @@ -1209,16 +1209,16 @@ static irqreturn_t fusb300_irq(int irq, void *_fusb300)
> }
>
> if (int_grp1 & FUSB300_IGR1_CX_CMDEND_INT)
> - printk(KERN_INFO "fusb300_cmdend\n");
> + pr_info("fusb300_cmdend\n");
>
>
> if (int_grp1 & FUSB300_IGR1_CX_OUT_INT) {
> - printk(KERN_INFO "fusb300_cxout\n");
> + pr_info("fusb300_cxout\n");
> fusb300_ep0out(fusb300);
> }
>
> if (int_grp1 & FUSB300_IGR1_CX_IN_INT) {
> - printk(KERN_INFO "fusb300_cxin\n");
> + pr_info("fusb300_cxin\n");
> fusb300_ep0in(fusb300);
> }
>
> diff --git a/drivers/usb/gadget/udc/goku_udc.c b/drivers/usb/gadget/udc/goku_udc.c
> index 3e1267d38774..4f225552861a 100644
> --- a/drivers/usb/gadget/udc/goku_udc.c
> +++ b/drivers/usb/gadget/udc/goku_udc.c
> @@ -1748,7 +1748,7 @@ static int goku_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> int retval;
>
> if (!pdev->irq) {
> - printk(KERN_ERR "Check PCI %s IRQ setup!\n", pci_name(pdev));
> + pr_err("Check PCI %s IRQ setup!\n", pci_name(pdev));
> retval = -ENODEV;
> goto err;
> }
> diff --git a/drivers/usb/gadget/udc/r8a66597-udc.h b/drivers/usb/gadget/udc/r8a66597-udc.h
> index 9a115caba661..fa4d62c32ea1 100644
> --- a/drivers/usb/gadget/udc/r8a66597-udc.h
> +++ b/drivers/usb/gadget/udc/r8a66597-udc.h
> @@ -247,7 +247,7 @@ static inline u16 get_xtal_from_pdata(struct r8a66597_platdata *pdata)
> clock = XTAL48;
> break;
> default:
> - printk(KERN_ERR "r8a66597: platdata clock is wrong.\n");
> + pr_err("r8a66597: platdata clock is wrong.\n");
> break;
> }
>
> --
> 2.11.0
>
--
Thanks,
Peter Chen
^ permalink raw reply
* Re: [PATCH v2 1/2] powerpc/rtas: Restrict RTAS requests from userspace
From: Andrew Donnellan @ 2020-12-08 23:23 UTC (permalink / raw)
To: Tyrel Datwyler, linuxppc-dev; +Cc: nathanl, leobras.c, stable, dja
In-Reply-To: <e58e8c42-d422-1bd7-ab38-9a1fb118fca4@linux.ibm.com>
On 9/12/20 5:59 am, Tyrel Datwyler wrote:
>> + { "ibm,open-errinct", -1, -1, -1, -1, -1 },
>
> There is a typo here. Should be ibm,open-errinjct.
>
> kernel: [ 1100.408626] sys_rtas: RTAS call blocked - exploit attempt?
> kernel: [ 1100.408631] sys_rtas: token=0x26, nargs=0 (called by errinjct)
>
> Which is producing this when trying to invoke the errinjct tool.
>
> I'll send a fixes patch out shortly.
*sigh*
Thanks for picking this up!
--
Andrew Donnellan OzLabs, ADL Canberra
ajd@linux.ibm.com IBM Australia Limited
^ permalink raw reply
* Re: [PATCH] arch: fix 'unexpected IRQ trap at vector' warnings
From: Thomas Gleixner @ 2020-12-08 23:01 UTC (permalink / raw)
To: Michael Ellerman, Enrico Weigelt, metux IT consult, linux-kernel
Cc: linux-s390, hpa, linux-parisc, deller, x86, linux-um,
James.Bottomley, mingo, paulus, richard, bp, linuxppc-dev, jdike,
anton.ivanov
In-Reply-To: <877dptt5av.fsf@mpe.ellerman.id.au>
On Tue, Dec 08 2020 at 13:11, Michael Ellerman wrote:
> "Enrico Weigelt, metux IT consult" <info@metux.net> writes:
>> All archs, except Alpha, print out the irq number in hex, but the message
>> looks like it was a decimal number, which is quite confusing. Fixing this
>> by adding "0x" prefix.
>
> Arguably decimal would be better, /proc/interrupts and /proc/irq/ both
> use decimal.
>
> The whole message is very dated IMO, these days the number it prints is
> (possibly) virtualised via IRQ domains, ie. it's not necessarily a
> "vector" if that even makes sense on all arches). Arguably "trap" is the
> wrong term on some arches too.
>
> So it would be better reworded entirely IMO, and also switched to
> decimal to match other sources of information on interrupts.
So much for the theory.
The printk originates from the very early days of i386 Linux where it
was called from the low level entry code when there was no interrupt
assigned to a vector, which is an x86'ism.
That was copied to other architectures without actually thinking about
whether the vector concept made sense on that architecture and at some
point it got completely bonkers because it moved to core code without
thought.
There are a few situations why it is invoked or not:
1) The original x86 usage is not longer using it because it complains
rightfully about a vector being raised which has no interrupt
descriptor associated to it. So the original reason for naming it
vector is gone long ago. It emits:
pr_emerg_ratelimited("%s: %d.%u No irq handler for vector\n",
__func__, smp_processor_id(), vector);
Directly from the x86 C entry point without ever invoking that
function. Pretty popular error message due to some AMD BIOS
wreckage. :)
2) It's invoked when there is an interrupt descriptor installed but
not configured/requested. In that case some architectures need to
ack it in order not to block further interrupt delivery. In that
case 'vector is bogus' and really want's to be 'irqnr' or such
because there is a Linux virq number associated to it.
3) It's invoked from __handle_domain_irq() when the 'hwirq' which is
handed in by the caller does not resolve to a mapped Linux
interrupt which is pretty much the same as the x86 situation above
in #1, but it prints useless data.
It prints 'irq' which is invalid but it does not print the really
interesting 'hwirq' which was handed in by the caller and did
not resolve.
In this case the Linux irq number is uninteresting as it is known
to be invalid and simply is not mapped and therefore does not
exist.
This has to print out 'hwirq' which is kinda the equivalent to the
original 'vector' message.
4) It's invoked from the dummy irq chip which is installed for a
couple of truly virtual interrupts where the invocation of
dummy_irq_chip::irq_ack() is indicating wreckage.
In that case the Linux irq number is the thing which is printed.
So no. It's not just inconsistent it's in some places outright
wrong. What we really want is:
ack_bad_irq(int hwirq, int virq)
{
if (hwirq >= 0)
print_useful_info(hwirq);
if (virq > 0)
print_useful_info(virq);
arch_try_to_ack(hwirq, virq);
}
for this to make sense. Just fixing the existing printk() to be less
wrong is not really an improvement.
Thanks,
tglx
^ permalink raw reply
* Re: [PATCH v2 01/17] ibmvfc: add vhost fields and defaults for MQ enablement
From: Tyrel Datwyler @ 2020-12-08 22:37 UTC (permalink / raw)
To: Hannes Reinecke, Brian King, james.bottomley
Cc: brking, linuxppc-dev, linux-scsi, martin.petersen, linux-kernel
In-Reply-To: <6ce79011-d288-7a49-3d51-262da58d8486@suse.de>
On 12/7/20 3:56 AM, Hannes Reinecke wrote:
> On 12/4/20 3:26 PM, Brian King wrote:
>> On 12/2/20 11:27 AM, Tyrel Datwyler wrote:
>>> On 12/2/20 7:14 AM, Brian King wrote:
>>>> On 12/1/20 6:53 PM, Tyrel Datwyler wrote:
>>>>> Introduce several new vhost fields for managing MQ state of the adapter
>>>>> as well as initial defaults for MQ enablement.
>>>>>
>>>>> Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
>>>>> ---
>>>>> drivers/scsi/ibmvscsi/ibmvfc.c | 9 ++++++++-
>>>>> drivers/scsi/ibmvscsi/ibmvfc.h | 13 +++++++++++--
>>>>> 2 files changed, 19 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
>>>>> index 42e4d35e0d35..f1d677a7423d 100644
>>>>> --- a/drivers/scsi/ibmvscsi/ibmvfc.c
>>>>> +++ b/drivers/scsi/ibmvscsi/ibmvfc.c
>>>>> @@ -5161,12 +5161,13 @@ static int ibmvfc_probe(struct vio_dev *vdev, const
>>>>> struct vio_device_id *id)
>>>>> }
>>>>> shost->transportt = ibmvfc_transport_template;
>>>>> - shost->can_queue = max_requests;
>>>>> + shost->can_queue = (max_requests / IBMVFC_SCSI_HW_QUEUES);
>>>>
>>>> This doesn't look right. can_queue is the SCSI host queue depth, not the MQ
>>>> queue depth.
>>>
>>> Our max_requests is the total number commands allowed across all queues. From
>>> what I understand is can_queue is the total number of commands in flight allowed
>>> for each hw queue.
>>>
>>> /*
>>> * In scsi-mq mode, the number of hardware queues supported by the LLD.
>>> *
>>> * Note: it is assumed that each hardware queue has a queue depth of
>>> * can_queue. In other words, the total queue depth per host
>>> * is nr_hw_queues * can_queue. However, for when host_tagset is set,
>>> * the total queue depth is can_queue.
>>> */
>>>
>>> We currently don't use the host wide shared tagset.
>>
>> Ok. I missed that bit... In that case, since we allocate by default only 100
>> event structs. If we slice that across IBMVFC_SCSI_HW_QUEUES (16) queues, then
>> we end up with only about 6 commands that can be outstanding per queue,
>> which is going to really hurt performance... I'd suggest bumping up
>> IBMVFC_MAX_REQUESTS_DEFAULT from 100 to 1000 as a starting point.
>>
> Before doing that I'd rather use the host-wide shared tagset.
> Increasing the number of requests will increase the memory footprint of the
> driver (as each request will be statically allocated).
>
In the case where we use host-wide how do I determine the queue depth per
hardware queue? Is is hypothetically can_queue or is it (can_queue /
nr_hw_queues)? We want to allocate an event pool per-queue which made sense
without host-wide tags since the queue depth per hw queue is exactly can_queue.
-Tyrel
^ permalink raw reply
* [PATCH v2 1/1] powerpc/kvm: Fix mask size for emulated msgsndp
From: Leonardo Bras @ 2020-12-08 21:57 UTC (permalink / raw)
To: Paul Mackerras, Michael Ellerman, Benjamin Herrenschmidt
Cc: Leonardo Bras, linuxppc-dev, linux-kernel, kvm-ppc
According to ISAv3.1 and ISAv3.0b, the msgsndp is described to split RB in:
msgtype <- (RB) 32:36
payload <- (RB) 37:63
t <- (RB) 57:63
The current way of getting 'msgtype', and 't' is missing their MSB:
msgtype: ((arg >> 27) & 0xf) : Gets (RB) 33:36, missing bit 32
t: (arg &= 0x3f) : Gets (RB) 58:63, missing bit 57
Fixes this by applying the correct mask.
Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
---
Changes since v1:
- Commit message 's/LSB/MSB/', because ISA ordering is big-endian.
arch/powerpc/kvm/book3s_hv.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index e3b1839fc251..5af0a429cee8 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -1241,9 +1241,9 @@ static int kvmppc_emulate_doorbell_instr(struct kvm_vcpu *vcpu)
switch (get_xop(inst)) {
case OP_31_XOP_MSGSNDP:
arg = kvmppc_get_gpr(vcpu, rb);
- if (((arg >> 27) & 0xf) != PPC_DBELL_SERVER)
+ if (((arg >> 27) & 0x1f) != PPC_DBELL_SERVER)
break;
- arg &= 0x3f;
+ arg &= 0x7f;
if (arg >= kvm->arch.emul_smt_mode)
break;
tvcpu = kvmppc_find_vcpu(kvm, vcpu->vcpu_id - thr + arg);
@@ -1256,7 +1256,7 @@ static int kvmppc_emulate_doorbell_instr(struct kvm_vcpu *vcpu)
break;
case OP_31_XOP_MSGCLRP:
arg = kvmppc_get_gpr(vcpu, rb);
- if (((arg >> 27) & 0xf) != PPC_DBELL_SERVER)
+ if (((arg >> 27) & 0x1f) != PPC_DBELL_SERVER)
break;
vcpu->arch.vcore->dpdes = 0;
vcpu->arch.doorbell_request = 0;
--
2.25.4
^ permalink raw reply related
* Re: [PATCH 14/20] ethernet: ucc_geth: don't statically allocate eight ucc_geth_info
From: Rasmus Villemoes @ 2020-12-08 21:17 UTC (permalink / raw)
To: Christophe Leroy, Li Yang, David S. Miller, Jakub Kicinski
Cc: Vladimir Oltean, Zhao Qiang, linuxppc-dev, linux-kernel, netdev
In-Reply-To: <8259bec3-9343-82e3-a420-a8170cf922a4@csgroup.eu>
On 08/12/2020 16.13, Christophe Leroy wrote:
>
>
> Le 05/12/2020 à 20:17, Rasmus Villemoes a écrit :
>> @@ -3714,25 +3712,23 @@ static int ucc_geth_probe(struct
>> platform_device* ofdev)
>> if ((ucc_num < 0) || (ucc_num > 7))
>> return -ENODEV;
>> - ug_info = &ugeth_info[ucc_num];
>> - if (ug_info == NULL) {
>> - if (netif_msg_probe(&debug))
>> - pr_err("[%d] Missing additional data!\n", ucc_num);
>> - return -ENODEV;
>> - }
>> + ug_info = kmalloc(sizeof(*ug_info), GFP_KERNEL);
>
> Could we use dev_kmalloc() instead, to avoid the freeing on the wait out
> and the err_free_info: path ?
Perhaps, but I don't think mixing ordinary kmalloc() with devm_ versions
in the same driver is a good idea - IIRC there are at least some rules
to obey if one does that, but I don't remember and can't find what they are.
Rasmus
^ permalink raw reply
* Re: [PATCH v6 0/5] PCI: Unify ECAM constants in native PCI Express drivers
From: Michael Walle @ 2020-12-08 21:11 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: kw, heiko, shawn.lin, paulus, thomas.petazzoni, jonnyc, toan,
will, robh, lorenzo.pieralisi, michal.simek, linux-rockchip,
bcm-kernel-feedback-list, linux-arm-kernel, linux-pci, Qian Cai,
rjui, f.fainelli, linux-rpi-kernel, Jonathan.Cameron, bhelgaas,
jonathan.derrick, sbranden, wangzhou1, rrichter, linuxppc-dev,
nsaenzjulienne
In-Reply-To: <20201208210613.GA2420289@bjorn-Precision-5520>
Am 2020-12-08 22:06, schrieb Bjorn Helgaas:
> [+cc Qian]
>
> On Tue, Dec 08, 2020 at 04:41:50PM +0100, Michael Walle wrote:
>> Hi Lorenzo, Krzysztof,
>>
>> >On Sun, 29 Nov 2020 23:07:38 +0000, Krzysztof Wilczyński wrote:
>> >> Unify ECAM-related constants into a single set of standard constants
>> >> defining memory address shift values for the byte-level address that can
>> >> be used when accessing the PCI Express Configuration Space, and then
>> >> move native PCI Express controller drivers to use newly introduced
>> >> definitions retiring any driver-specific ones.
>> >>
>> >> The ECAM ("Enhanced Configuration Access Mechanism") is defined by the
>> >> PCI Express specification (see PCI Express Base Specification, Revision
>> >> 5.0, Version 1.0, Section 7.2.2, p. 676), thus most hardware should
>> >> implement it the same way.
>> >>
>> >> [...]
>> >
>> >Applied to pci/ecam, thanks!
>> >
>> >[1/5] PCI: Unify ECAM constants in native PCI Express drivers
>> > https://git.kernel.org/lpieralisi/pci/c/f3c07cf692
>> >[2/5] PCI: thunder-pem: Add constant for custom ".bus_shift" initialiser
>> > https://git.kernel.org/lpieralisi/pci/c/3c38579263
>> >[3/5] PCI: iproc: Convert to use the new ECAM constants
>> > https://git.kernel.org/lpieralisi/pci/c/333ec9d3cc
>> >[4/5] PCI: vmd: Update type of the __iomem pointers
>> > https://git.kernel.org/lpieralisi/pci/c/89094c12ea
>> >[5/5] PCI: xgene: Removed unused ".bus_shift" initialisers from pci-xgene.c
>> > https://git.kernel.org/lpieralisi/pci/c/3dc62532a5
>>
>> Patch 1/5 breaks LS1028A boards:
>
> I temporarily dropped this series while we figure out what went wrong
> here.
Thanks, let me know if I can test something on the board.
-michael
^ permalink raw reply
* Re: [PATCH v6 0/5] PCI: Unify ECAM constants in native PCI Express drivers
From: Bjorn Helgaas @ 2020-12-08 21:06 UTC (permalink / raw)
To: Michael Walle
Cc: kw, heiko, shawn.lin, paulus, thomas.petazzoni, jonnyc, toan,
will, robh, lorenzo.pieralisi, michal.simek, linux-rockchip,
bcm-kernel-feedback-list, linux-arm-kernel, linux-pci, Qian Cai,
rjui, f.fainelli, linux-rpi-kernel, Jonathan.Cameron, bhelgaas,
jonathan.derrick, sbranden, wangzhou1, rrichter, linuxppc-dev,
nsaenzjulienne
In-Reply-To: <20201208154150.20978-1-michael@walle.cc>
[+cc Qian]
On Tue, Dec 08, 2020 at 04:41:50PM +0100, Michael Walle wrote:
> Hi Lorenzo, Krzysztof,
>
> >On Sun, 29 Nov 2020 23:07:38 +0000, Krzysztof Wilczyński wrote:
> >> Unify ECAM-related constants into a single set of standard constants
> >> defining memory address shift values for the byte-level address that can
> >> be used when accessing the PCI Express Configuration Space, and then
> >> move native PCI Express controller drivers to use newly introduced
> >> definitions retiring any driver-specific ones.
> >>
> >> The ECAM ("Enhanced Configuration Access Mechanism") is defined by the
> >> PCI Express specification (see PCI Express Base Specification, Revision
> >> 5.0, Version 1.0, Section 7.2.2, p. 676), thus most hardware should
> >> implement it the same way.
> >>
> >> [...]
> >
> >Applied to pci/ecam, thanks!
> >
> >[1/5] PCI: Unify ECAM constants in native PCI Express drivers
> > https://git.kernel.org/lpieralisi/pci/c/f3c07cf692
> >[2/5] PCI: thunder-pem: Add constant for custom ".bus_shift" initialiser
> > https://git.kernel.org/lpieralisi/pci/c/3c38579263
> >[3/5] PCI: iproc: Convert to use the new ECAM constants
> > https://git.kernel.org/lpieralisi/pci/c/333ec9d3cc
> >[4/5] PCI: vmd: Update type of the __iomem pointers
> > https://git.kernel.org/lpieralisi/pci/c/89094c12ea
> >[5/5] PCI: xgene: Removed unused ".bus_shift" initialisers from pci-xgene.c
> > https://git.kernel.org/lpieralisi/pci/c/3dc62532a5
>
> Patch 1/5 breaks LS1028A boards:
I temporarily dropped this series while we figure out what went wrong
here.
Bjorn
^ 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