* Re: [PATCH] powerpc/bug: Remove specific powerpc BUG_ON()
From: Segher Boessenkool @ 2021-02-11 12:22 UTC (permalink / raw)
To: Nicholas Piggin; +Cc: linux-kernel, Paul Mackerras, linuxppc-dev
In-Reply-To: <1613036567.zvyupcz926.astroid@bobo.none>
On Thu, Feb 11, 2021 at 08:04:55PM +1000, Nicholas Piggin wrote:
> Excerpts from Christophe Leroy's message of February 11, 2021 5:41 pm:
> > As modern powerpc implement branch folding, that's even more efficient.
Ah, it seems you mean what Arm calls branch folding. Sure, power4
already did that, and this has not changed.
> I think POWER will speculate conditional traps as non faulting always
> so it should be just as good if not better than the branch.
Right, these are not branch instructions, so are not branch predicted;
all trap instructions are assumed to fall through, like all other
non-branch instructions.
Segher
^ permalink raw reply
* Re: [PATCH] powerpc/bug: Remove specific powerpc BUG_ON()
From: Christophe Leroy @ 2021-02-11 12:26 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linux-kernel, npiggin, Paul Mackerras, linuxppc-dev
In-Reply-To: <20210211114910.GA28121@gate.crashing.org>
Le 11/02/2021 à 12:49, Segher Boessenkool a écrit :
> On Thu, Feb 11, 2021 at 07:41:52AM +0000, Christophe Leroy wrote:
>> powerpc BUG_ON() is based on using twnei or tdnei instruction,
>> which obliges gcc to format the condition into a 0 or 1 value
>> in a register.
>
> Huh? Why is that?
>
> Will it work better if this used __builtin_trap? Or does the kernel only
> detect very specific forms of trap instructions?
>
>> By using a generic implementation, gcc will generate a branch
>> to the unconditional trap generated by BUG().
>
> That is many more instructions than ideal.
>
>> As modern powerpc implement branch folding, that's even more efficient.
>
> What PowerPC cpus implement branch folding? I know none.
Extract from powerpc mpc8323 reference manual:
High instruction and data throughput
— Zero-cycle branch capability (branch folding)
— Programmable static branch prediction on unresolved conditional branches
— Two integer units with enhanced multipliers in thee300c2 for increased integer instruction
throughput and a maximum two-cycle latency for multiply instructions
— Instruction fetch unit capable of fetching two instructions per clock from the instruction cache
— A six-entry instruction queue (IQ) that provides lookahead capability
— Independent pipelines with feed-forwarding that reduces data dependencies in hardware
— 16-Kbyte, four-way set-associative instruction and data caches on the e300c2.
— Cache write-back or write-through operation programmable on a per-page or per-block basis
— Features for instruction and data cache locking and protection
— BPU that performs CR lookahead operations
— Address translation facilities for 4-Kbyte page size, variable block size, and 256-Mbyte
segment size
— A 64-entry, two-way, set-associative ITLB and DTLB
— Eight-entry data and instruction BAT arrays providing 128-Kbyte to 256-Mbyte blocks
— Software table search operations and updates supported through fast trap mechanism
— 52-bit virtual address; 32-bit physical address
Christophe
^ permalink raw reply
* Re: [PATCH] powerpc/bug: Remove specific powerpc BUG_ON()
From: Segher Boessenkool @ 2021-02-11 12:39 UTC (permalink / raw)
To: Christophe Leroy; +Cc: linux-kernel, npiggin, Paul Mackerras, linuxppc-dev
In-Reply-To: <3b7f3a1e-0355-b6d4-14cd-300bf4d3629a@csgroup.eu>
On Thu, Feb 11, 2021 at 01:26:12PM +0100, Christophe Leroy wrote:
> >What PowerPC cpus implement branch folding? I know none.
>
> Extract from powerpc mpc8323 reference manual:
> — Zero-cycle branch capability (branch folding)
Yeah, this is not what is traditionally called branch folding (which
stores the instruction being branched to in some cache, originally the
instruction cache itself; somewhat similar (but different) to the BTIC
on 750). Overloaded terminology :-)
6xx/7xx CPUs had the branch execution unit in the frontend, and it would
not issue a branch at all if it could be resolved then already. Power4
and later predict all branches, and most are not issued at all (those
that do need to be executed, like bdnz, are). At completion time it is
checked if the prediction was correct (and corrective action is taken if
not).
Segher
^ permalink raw reply
* [PATCH 3/6] powerpc/64s: Use htab_convert_pte_flags() in hash__mark_rodata_ro()
From: Michael Ellerman @ 2021-02-11 13:51 UTC (permalink / raw)
To: linuxppc-dev; +Cc: aneesh.kumar
In-Reply-To: <20210211135130.3474832-1-mpe@ellerman.id.au>
In hash__mark_rodata_ro() we pass the raw PP_RXXX value to
hash__change_memory_range(). That has the effect of setting the key to
zero, because PP_RXXX contains no key value.
Fix it by using htab_convert_pte_flags(), which knows how to convert a
pgprot into a pp value, including the key.
Fixes: d94b827e89dc ("powerpc/book3s64/kuap: Use Key 3 for kernel mapping with hash translation")
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/mm/book3s64/hash_pgtable.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/mm/book3s64/hash_pgtable.c b/arch/powerpc/mm/book3s64/hash_pgtable.c
index 567e0c6b3978..03819c259f0a 100644
--- a/arch/powerpc/mm/book3s64/hash_pgtable.c
+++ b/arch/powerpc/mm/book3s64/hash_pgtable.c
@@ -428,12 +428,14 @@ static bool hash__change_memory_range(unsigned long start, unsigned long end,
void hash__mark_rodata_ro(void)
{
- unsigned long start, end;
+ unsigned long start, end, pp;
start = (unsigned long)_stext;
end = (unsigned long)__init_begin;
- WARN_ON(!hash__change_memory_range(start, end, PP_RXXX));
+ pp = htab_convert_pte_flags(pgprot_val(PAGE_KERNEL_ROX), HPTE_USE_KERNEL_KEY);
+
+ WARN_ON(!hash__change_memory_range(start, end, pp));
}
void hash__mark_initmem_nx(void)
--
2.25.1
^ permalink raw reply related
* [PATCH 1/6] powerpc/mm/64s: Add _PAGE_KERNEL_ROX
From: Michael Ellerman @ 2021-02-11 13:51 UTC (permalink / raw)
To: linuxppc-dev; +Cc: aneesh.kumar
In the past we had a fallback definition for _PAGE_KERNEL_ROX, but we
removed that in commit d82fd29c5a8c ("powerpc/mm: Distribute platform
specific PAGE and PMD flags and definitions") and added definitions
for each MMU family.
However we missed adding a definition for 64s, which was not really a
bug because it's currently not used.
But we'd like to use PAGE_KERNEL_ROX in a future patch so add a
definition now.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/include/asm/book3s/64/pgtable.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
index a39886681629..1358fae01d04 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -116,6 +116,7 @@
*/
#define _PAGE_KERNEL_RW (_PAGE_PRIVILEGED | _PAGE_RW | _PAGE_DIRTY)
#define _PAGE_KERNEL_RO (_PAGE_PRIVILEGED | _PAGE_READ)
+#define _PAGE_KERNEL_ROX (_PAGE_PRIVILEGED | _PAGE_READ | _PAGE_EXEC)
#define _PAGE_KERNEL_RWX (_PAGE_PRIVILEGED | _PAGE_DIRTY | \
_PAGE_RW | _PAGE_EXEC)
/*
--
2.25.1
^ permalink raw reply related
* [PATCH 2/6] powerpc/pseries: Add key to flags in pSeries_lpar_hpte_updateboltedpp()
From: Michael Ellerman @ 2021-02-11 13:51 UTC (permalink / raw)
To: linuxppc-dev; +Cc: aneesh.kumar
In-Reply-To: <20210211135130.3474832-1-mpe@ellerman.id.au>
The flags argument to plpar_pte_protect() (aka. H_PROTECT), includes
the key in bits 9-13, but currently we always set those bits to zero.
In the past that hasn't been a problem because we always used key 0
for the kernel, and updateboltedpp() is only used for kernel mappings.
However since commit d94b827e89dc ("powerpc/book3s64/kuap: Use Key 3
for kernel mapping with hash translation") we are now inadvertently
changing the key (to zero) when we call plpar_pte_protect().
That hasn't broken anything because updateboltedpp() is only used for
STRICT_KERNEL_RWX, which is currently disabled on 64s due to other
bugs.
But we want to fix that, so first we need to pass the key correctly to
plpar_pte_protect(). In the `newpp` value the low 3 bits of the key
are already in the correct spot, but the high 2 bits of the key need
to be shifted down.
Fixes: d94b827e89dc ("powerpc/book3s64/kuap: Use Key 3 for kernel mapping with hash translation")
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/platforms/pseries/lpar.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/pseries/lpar.c b/arch/powerpc/platforms/pseries/lpar.c
index 764170fdb0f7..8bbbddff7226 100644
--- a/arch/powerpc/platforms/pseries/lpar.c
+++ b/arch/powerpc/platforms/pseries/lpar.c
@@ -976,11 +976,13 @@ static void pSeries_lpar_hpte_updateboltedpp(unsigned long newpp,
slot = pSeries_lpar_hpte_find(vpn, psize, ssize);
BUG_ON(slot == -1);
- flags = newpp & 7;
+ flags = newpp & (HPTE_R_PP | HPTE_R_N);
if (mmu_has_feature(MMU_FTR_KERNEL_RO))
/* Move pp0 into bit 8 (IBM 55) */
flags |= (newpp & HPTE_R_PP0) >> 55;
+ flags |= ((newpp & HPTE_R_KEY_HI) >> 48) | (newpp & HPTE_R_KEY_LO);
+
lpar_rc = plpar_pte_protect(flags, slot, 0);
BUG_ON(lpar_rc != H_SUCCESS);
--
2.25.1
^ permalink raw reply related
* [PATCH 4/6] powerpc/mm/64s/hash: Factor out change_memory_range()
From: Michael Ellerman @ 2021-02-11 13:51 UTC (permalink / raw)
To: linuxppc-dev; +Cc: aneesh.kumar
In-Reply-To: <20210211135130.3474832-1-mpe@ellerman.id.au>
Pull the loop calling hpte_updateboltedpp() out of
hash__change_memory_range() into a helper function. We need it to be a
separate function for the next patch.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/mm/book3s64/hash_pgtable.c | 23 +++++++++++++++--------
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/mm/book3s64/hash_pgtable.c b/arch/powerpc/mm/book3s64/hash_pgtable.c
index 03819c259f0a..3663d3cdffac 100644
--- a/arch/powerpc/mm/book3s64/hash_pgtable.c
+++ b/arch/powerpc/mm/book3s64/hash_pgtable.c
@@ -400,10 +400,23 @@ EXPORT_SYMBOL_GPL(hash__has_transparent_hugepage);
#endif /* CONFIG_TRANSPARENT_HUGEPAGE */
#ifdef CONFIG_STRICT_KERNEL_RWX
+static void change_memory_range(unsigned long start, unsigned long end,
+ unsigned int step, unsigned long newpp)
+{
+ unsigned long idx;
+
+ pr_debug("Changing page protection on range 0x%lx-0x%lx, to 0x%lx, step 0x%x\n",
+ start, end, newpp, step);
+
+ for (idx = start; idx < end; idx += step)
+ /* Not sure if we can do much with the return value */
+ mmu_hash_ops.hpte_updateboltedpp(newpp, idx, mmu_linear_psize,
+ mmu_kernel_ssize);
+}
+
static bool hash__change_memory_range(unsigned long start, unsigned long end,
unsigned long newpp)
{
- unsigned long idx;
unsigned int step, shift;
shift = mmu_psize_defs[mmu_linear_psize].shift;
@@ -415,13 +428,7 @@ static bool hash__change_memory_range(unsigned long start, unsigned long end,
if (start >= end)
return false;
- pr_debug("Changing page protection on range 0x%lx-0x%lx, to 0x%lx, step 0x%x\n",
- start, end, newpp, step);
-
- for (idx = start; idx < end; idx += step)
- /* Not sure if we can do much with the return value */
- mmu_hash_ops.hpte_updateboltedpp(newpp, idx, mmu_linear_psize,
- mmu_kernel_ssize);
+ change_memory_range(start, end, step, newpp);
return true;
}
--
2.25.1
^ permalink raw reply related
* [PATCH 5/6] powerpc/mm/64s/hash: Add real-mode change_memory_range() for hash LPAR
From: Michael Ellerman @ 2021-02-11 13:51 UTC (permalink / raw)
To: linuxppc-dev; +Cc: aneesh.kumar
In-Reply-To: <20210211135130.3474832-1-mpe@ellerman.id.au>
When we enabled STRICT_KERNEL_RWX we received some reports of boot
failures when using the Hash MMU and running under phyp. The crashes
are intermittent, and often exhibit as a completely unresponsive
system, or possibly an oops.
One example, which was caught in xmon:
[ 14.068327][ T1] devtmpfs: mounted
[ 14.069302][ T1] Freeing unused kernel memory: 5568K
[ 14.142060][ T347] BUG: Unable to handle kernel instruction fetch
[ 14.142063][ T1] Run /sbin/init as init process
[ 14.142074][ T347] Faulting instruction address: 0xc000000000004400
cpu 0x2: Vector: 400 (Instruction Access) at [c00000000c7475e0]
pc: c000000000004400: exc_virt_0x4400_instruction_access+0x0/0x80
lr: c0000000001862d4: update_rq_clock+0x44/0x110
sp: c00000000c747880
msr: 8000000040001031
current = 0xc00000000c60d380
paca = 0xc00000001ec9de80 irqmask: 0x03 irq_happened: 0x01
pid = 347, comm = kworker/2:1
...
enter ? for help
[c00000000c747880] c0000000001862d4 update_rq_clock+0x44/0x110 (unreliable)
[c00000000c7478f0] c000000000198794 update_blocked_averages+0xb4/0x6d0
[c00000000c7479f0] c000000000198e40 update_nohz_stats+0x90/0xd0
[c00000000c747a20] c0000000001a13b4 _nohz_idle_balance+0x164/0x390
[c00000000c747b10] c0000000001a1af8 newidle_balance+0x478/0x610
[c00000000c747be0] c0000000001a1d48 pick_next_task_fair+0x58/0x480
[c00000000c747c40] c000000000eaab5c __schedule+0x12c/0x950
[c00000000c747cd0] c000000000eab3e8 schedule+0x68/0x120
[c00000000c747d00] c00000000016b730 worker_thread+0x130/0x640
[c00000000c747da0] c000000000174d50 kthread+0x1a0/0x1b0
[c00000000c747e10] c00000000000e0f0 ret_from_kernel_thread+0x5c/0x6c
This shows that CPU 2, which was idle, woke up and then appears to
randomly take an instruction fault on a completely valid area of
kernel text.
The cause turns out to be the call to hash__mark_rodata_ro(), late in
boot. Due to the way we layout text and rodata, that function actually
changes the permissions for all of text and rodata to read-only plus
execute.
To do the permission change we use a hypervisor call, H_PROTECT. On
phyp that appears to be implemented by briefly removing the mapping of
the kernel text, before putting it back with the updated permissions.
If any other CPU is executing during that window, it will see spurious
faults on the kernel text and/or data, leading to crashes.
To fix it we use stop machine to collect all other CPUs, and then have
them drop into real mode (MMU off), while we change the mapping. That
way they are unaffected by the mapping temporarily disappearing.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/mm/book3s64/hash_pgtable.c | 105 +++++++++++++++++++++++-
1 file changed, 104 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/mm/book3s64/hash_pgtable.c b/arch/powerpc/mm/book3s64/hash_pgtable.c
index 3663d3cdffac..01de985df2c4 100644
--- a/arch/powerpc/mm/book3s64/hash_pgtable.c
+++ b/arch/powerpc/mm/book3s64/hash_pgtable.c
@@ -8,6 +8,7 @@
#include <linux/sched.h>
#include <linux/mm_types.h>
#include <linux/mm.h>
+#include <linux/stop_machine.h>
#include <asm/sections.h>
#include <asm/mmu.h>
@@ -400,6 +401,19 @@ EXPORT_SYMBOL_GPL(hash__has_transparent_hugepage);
#endif /* CONFIG_TRANSPARENT_HUGEPAGE */
#ifdef CONFIG_STRICT_KERNEL_RWX
+
+struct change_memory_parms {
+ unsigned long start, end, newpp;
+ unsigned int step, nr_cpus, master_cpu;
+ atomic_t cpu_counter;
+};
+
+// We'd rather this was on the stack but it has to be in the RMO
+static struct change_memory_parms chmem_parms;
+
+// And therefore we need a lock to protect it from concurrent use
+static DEFINE_MUTEX(chmem_lock);
+
static void change_memory_range(unsigned long start, unsigned long end,
unsigned int step, unsigned long newpp)
{
@@ -414,6 +428,73 @@ static void change_memory_range(unsigned long start, unsigned long end,
mmu_kernel_ssize);
}
+static int notrace chmem_secondary_loop(struct change_memory_parms *parms)
+{
+ unsigned long msr, tmp, flags;
+ int *p;
+
+ p = &parms->cpu_counter.counter;
+
+ local_irq_save(flags);
+ __hard_EE_RI_disable();
+
+ asm volatile (
+ // Switch to real mode and leave interrupts off
+ "mfmsr %[msr] ;"
+ "li %[tmp], %[MSR_IR_DR] ;"
+ "andc %[tmp], %[msr], %[tmp] ;"
+ "mtmsrd %[tmp] ;"
+
+ // Tell the master we are in real mode
+ "1: "
+ "lwarx %[tmp], 0, %[p] ;"
+ "addic %[tmp], %[tmp], -1 ;"
+ "stwcx. %[tmp], 0, %[p] ;"
+ "bne- 1b ;"
+
+ // Spin until the counter goes to zero
+ "2: ;"
+ "lwz %[tmp], 0(%[p]) ;"
+ "cmpwi %[tmp], 0 ;"
+ "bne- 2b ;"
+
+ // Switch back to virtual mode
+ "mtmsrd %[msr] ;"
+
+ : // outputs
+ [msr] "=&r" (msr), [tmp] "=&b" (tmp), "+m" (*p)
+ : // inputs
+ [p] "b" (p), [MSR_IR_DR] "i" (MSR_IR | MSR_DR)
+ : // clobbers
+ "cc", "xer"
+ );
+
+ local_irq_restore(flags);
+
+ return 0;
+}
+
+static int change_memory_range_fn(void *data)
+{
+ struct change_memory_parms *parms = data;
+
+ if (parms->master_cpu != smp_processor_id())
+ return chmem_secondary_loop(parms);
+
+ // Wait for all but one CPU (this one) to call-in
+ while (atomic_read(&parms->cpu_counter) > 1)
+ barrier();
+
+ change_memory_range(parms->start, parms->end, parms->step, parms->newpp);
+
+ mb();
+
+ // Signal the other CPUs that we're done
+ atomic_dec(&parms->cpu_counter);
+
+ return 0;
+}
+
static bool hash__change_memory_range(unsigned long start, unsigned long end,
unsigned long newpp)
{
@@ -428,7 +509,29 @@ static bool hash__change_memory_range(unsigned long start, unsigned long end,
if (start >= end)
return false;
- change_memory_range(start, end, step, newpp);
+ if (firmware_has_feature(FW_FEATURE_LPAR)) {
+ mutex_lock(&chmem_lock);
+
+ chmem_parms.start = start;
+ chmem_parms.end = end;
+ chmem_parms.step = step;
+ chmem_parms.newpp = newpp;
+ chmem_parms.master_cpu = smp_processor_id();
+
+ cpus_read_lock();
+
+ atomic_set(&chmem_parms.cpu_counter, num_online_cpus());
+
+ // Ensure state is consistent before we call the other CPUs
+ mb();
+
+ stop_machine_cpuslocked(change_memory_range_fn, &chmem_parms,
+ cpu_online_mask);
+
+ cpus_read_unlock();
+ mutex_unlock(&chmem_lock);
+ } else
+ change_memory_range(start, end, step, newpp);
return true;
}
--
2.25.1
^ permalink raw reply related
* [PATCH 6/6] powerpc/mm/64s: Allow STRICT_KERNEL_RWX again
From: Michael Ellerman @ 2021-02-11 13:51 UTC (permalink / raw)
To: linuxppc-dev; +Cc: aneesh.kumar
In-Reply-To: <20210211135130.3474832-1-mpe@ellerman.id.au>
We have now fixed the known bugs in STRICT_KERNEL_RWX for Book3S
64-bit Hash and Radix MMUs, see preceding commits, so allow the
option to be selected again.
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
arch/powerpc/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 107bb4319e0e..7d6080afbad6 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -135,7 +135,7 @@ config PPC
select ARCH_HAS_MEMBARRIER_CALLBACKS
select ARCH_HAS_MEMBARRIER_SYNC_CORE
select ARCH_HAS_SCALED_CPUTIME if VIRT_CPU_ACCOUNTING_NATIVE && PPC_BOOK3S_64
- select ARCH_HAS_STRICT_KERNEL_RWX if (PPC32 && !HIBERNATION)
+ select ARCH_HAS_STRICT_KERNEL_RWX if ((PPC_BOOK3S_64 || PPC32) && !HIBERNATION)
select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
select ARCH_HAS_UACCESS_FLUSHCACHE
select ARCH_HAS_COPY_MC if PPC64
--
2.25.1
^ permalink raw reply related
* Re: [PATCH] powerpc/bug: Remove specific powerpc BUG_ON()
From: Christophe Leroy @ 2021-02-11 14:09 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linux-kernel, npiggin, Paul Mackerras, linuxppc-dev
In-Reply-To: <20210211114910.GA28121@gate.crashing.org>
Le 11/02/2021 à 12:49, Segher Boessenkool a écrit :
> On Thu, Feb 11, 2021 at 07:41:52AM +0000, Christophe Leroy wrote:
>> powerpc BUG_ON() is based on using twnei or tdnei instruction,
>> which obliges gcc to format the condition into a 0 or 1 value
>> in a register.
>
> Huh? Why is that?
>
> Will it work better if this used __builtin_trap? Or does the kernel only
> detect very specific forms of trap instructions?
We already made a try with __builtin_trap() 1,5 year ago, see
https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20510ce03cc9463f1c9e743c1d93b939de501b53.1566219503.git.christophe.leroy@c-s.fr/
The main problems encountered are:
- It is only possible to use it for BUG_ON, not for WARN_ON because GCC considers it as noreturn. Is
there any workaround ?
- The kernel (With CONFIG_DEBUG_BUGVERBOSE) needs to be able to identify the source file and line
corresponding to the trap. How can that be done with __builtin_trap() ?
Christophe
^ permalink raw reply
* Re: [PATCH] powerpc/bug: Remove specific powerpc BUG_ON()
From: Segher Boessenkool @ 2021-02-11 14:30 UTC (permalink / raw)
To: Christophe Leroy; +Cc: linux-kernel, npiggin, Paul Mackerras, linuxppc-dev
In-Reply-To: <6126ca14-419a-9e15-7ffa-b295f26a552e@csgroup.eu>
On Thu, Feb 11, 2021 at 03:09:43PM +0100, Christophe Leroy wrote:
> Le 11/02/2021 à 12:49, Segher Boessenkool a écrit :
> >On Thu, Feb 11, 2021 at 07:41:52AM +0000, Christophe Leroy wrote:
> >>powerpc BUG_ON() is based on using twnei or tdnei instruction,
> >>which obliges gcc to format the condition into a 0 or 1 value
> >>in a register.
> >
> >Huh? Why is that?
> >
> >Will it work better if this used __builtin_trap? Or does the kernel only
> >detect very specific forms of trap instructions?
>
> We already made a try with __builtin_trap() 1,5 year ago, see
> https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20510ce03cc9463f1c9e743c1d93b939de501b53.1566219503.git.christophe.leroy@c-s.fr/
>
> The main problems encountered are:
> - It is only possible to use it for BUG_ON, not for WARN_ON because GCC
> considers it as noreturn. Is there any workaround ?
A trap is noreturn by definition:
-- Built-in Function: void __builtin_trap (void)
This function causes the program to exit abnormally.
> - The kernel (With CONFIG_DEBUG_BUGVERBOSE) needs to be able to identify
> the source file and line corresponding to the trap. How can that be done
> with __builtin_trap() ?
The DWARF debug info should be sufficient. Perhaps you can post-process
some way?
You can create a trap that falls through yourself (by having a trap-on
condition with a condition that is always true, but make the compiler
not see that). This isn't efficient though.
Could you file a feature request (in bugzilla)? It is probably useful
for generic code as well, but we could implement this for powerpc only
if needed.
Segher
^ permalink raw reply
* Re: [PATCH] powerpc/bug: Remove specific powerpc BUG_ON()
From: Christophe Leroy @ 2021-02-11 15:30 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linux-kernel, npiggin, Paul Mackerras, linuxppc-dev
In-Reply-To: <20210211143059.GE28121@gate.crashing.org>
Le 11/02/2021 à 15:30, Segher Boessenkool a écrit :
> On Thu, Feb 11, 2021 at 03:09:43PM +0100, Christophe Leroy wrote:
>> Le 11/02/2021 à 12:49, Segher Boessenkool a écrit :
>>> On Thu, Feb 11, 2021 at 07:41:52AM +0000, Christophe Leroy wrote:
>>>> powerpc BUG_ON() is based on using twnei or tdnei instruction,
>>>> which obliges gcc to format the condition into a 0 or 1 value
>>>> in a register.
>>>
>>> Huh? Why is that?
>>>
>>> Will it work better if this used __builtin_trap? Or does the kernel only
>>> detect very specific forms of trap instructions?
>>
>> We already made a try with __builtin_trap() 1,5 year ago, see
>> https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20510ce03cc9463f1c9e743c1d93b939de501b53.1566219503.git.christophe.leroy@c-s.fr/
>>
>> The main problems encountered are:
>> - It is only possible to use it for BUG_ON, not for WARN_ON because GCC
>> considers it as noreturn. Is there any workaround ?
>
> A trap is noreturn by definition:
>
> -- Built-in Function: void __builtin_trap (void)
> This function causes the program to exit abnormally.
>
>> - The kernel (With CONFIG_DEBUG_BUGVERBOSE) needs to be able to identify
>> the source file and line corresponding to the trap. How can that be done
>> with __builtin_trap() ?
>
> The DWARF debug info should be sufficient. Perhaps you can post-process
> some way?
>
> You can create a trap that falls through yourself (by having a trap-on
> condition with a condition that is always true, but make the compiler
> not see that). This isn't efficient though.
>
> Could you file a feature request (in bugzilla)? It is probably useful
> for generic code as well, but we could implement this for powerpc only
> if needed.
>
Ok I will.
For sure using __builtin_trap() would be the best.
unsigned long test1(unsigned long msr)
{
WARN_ON((msr & (MSR_DR | MSR_IR | MSR_RI)) != (MSR_DR | MSR_IR | MSR_RI));
return msr;
}
unsigned long test2(unsigned long msr)
{
if ((msr & (MSR_DR | MSR_IR | MSR_RI)) != (MSR_DR | MSR_IR | MSR_RI))
__builtin_trap();
return msr;
}
000003c0 <test1>:
3c0: 70 69 00 32 andi. r9,r3,50
3c4: 69 29 00 32 xori r9,r9,50
3c8: 31 49 ff ff addic r10,r9,-1
3cc: 7d 2a 49 10 subfe r9,r10,r9
3d0: 0f 09 00 00 twnei r9,0
3d4: 4e 80 00 20 blr
000003d8 <test2>:
3d8: 70 69 00 32 andi. r9,r3,50
3dc: 0f 09 00 32 twnei r9,50
3e0: 4e 80 00 20 blr
Christophe
^ permalink raw reply
* Fwd: Re: [PATCH v17 02/10] of: Add a common kexec FDT setup function
From: Lakshmi Ramasubramanian @ 2021-02-11 17:42 UTC (permalink / raw)
To: Rob Herring, Thiago Jung Bauermann, Mimi Zohar, linux-integrity,
linux-arm-kernel, linux-integrity, linuxppc-dev
In-Reply-To: <202102120032.Bv0MoYv7-lkp@intel.com>
[-- Attachment #1: Type: text/plain, Size: 7614 bytes --]
Hi Rob,
[PATCH] powerpc: Rename kexec elfcorehdr_addr to elf_headers_mem
This change causes build problem for x86_64 architecture (please see the
mail from kernel test bot below) since arch/x86/include/asm/kexec.h uses
"elf_load_addr" for the ELF header buffer address and not "elf_headers_mem".
struct kimage_arch {
...
/* Core ELF header buffer */
void *elf_headers;
unsigned long elf_headers_sz;
unsigned long elf_load_addr;
};
I am thinking of limiting of_kexec_alloc_and_setup_fdt() to ARM64 and
PPC64 since they are the only ones using this function now.
#if defined(CONFIG_ARM64) && defined(CONFIG_PPC64)
void *of_kexec_alloc_and_setup_fdt(const struct kimage *image,
unsigned long initrd_load_addr,
unsigned long initrd_len,
const char *cmdline)
{
...
}
#endif /* defined(CONFIG_ARM64) && defined(CONFIG_PPC64) */
Please let me know if you have any concerns.
thanks,
-lakshmi
-------- Forwarded Message --------
Subject: Re: [PATCH v17 02/10] of: Add a common kexec FDT setup function
Date: Fri, 12 Feb 2021 00:50:20 +0800
From: kernel test robot <lkp@intel.com>
To: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
CC: kbuild-all@lists.01.org
Hi Lakshmi,
I love your patch! Yet something to improve:
[auto build test ERROR on integrity/next-integrity]
[also build test ERROR on v5.11-rc7 next-20210211]
[cannot apply to powerpc/next robh/for-next arm64/for-next/core]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Lakshmi-Ramasubramanian/Carry-forward-IMA-measurement-log-on-kexec-on-ARM64/20210211-071924
base:
https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git
next-integrity
config: x86_64-randconfig-m001-20210211 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
#
https://github.com/0day-ci/linux/commit/12ae86067d115b84092353109e8798693d102f0d
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Lakshmi-Ramasubramanian/Carry-forward-IMA-measurement-log-on-kexec-on-ARM64/20210211-071924
git checkout 12ae86067d115b84092353109e8798693d102f0d
# save the attached .config to linux build tree
make W=1 ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
drivers/of/kexec.c: In function 'of_kexec_alloc_and_setup_fdt':
>> drivers/of/kexec.c:183:17: error: 'const struct kimage_arch' has no member named 'elf_headers_mem'; did you mean 'elf_headers_sz'?
183 | image->arch.elf_headers_mem,
| ^~~~~~~~~~~~~~~
| elf_headers_sz
drivers/of/kexec.c:192:42: error: 'const struct kimage_arch' has no
member named 'elf_headers_mem'; did you mean 'elf_headers_sz'?
192 | ret = fdt_add_mem_rsv(fdt, image->arch.elf_headers_mem,
| ^~~~~~~~~~~~~~~
| elf_headers_sz
vim +183 drivers/of/kexec.c
65
66 /*
67 * of_kexec_alloc_and_setup_fdt - Alloc and setup a new
Flattened Device Tree
68 *
69 * @image: kexec image being loaded.
70 * @initrd_load_addr: Address where the next initrd will be loaded.
71 * @initrd_len: Size of the next initrd, or 0 if there will be
none.
72 * @cmdline: Command line for the next kernel, or NULL if there
will
73 * be none.
74 *
75 * Return: fdt on success, or NULL errno on error.
76 */
77 void *of_kexec_alloc_and_setup_fdt(const struct kimage *image,
78 unsigned long initrd_load_addr,
79 unsigned long initrd_len,
80 const char *cmdline)
81 {
82 void *fdt;
83 int ret, chosen_node;
84 const void *prop;
85 unsigned long fdt_size;
86
87 fdt_size = fdt_totalsize(initial_boot_params) +
88 (cmdline ? strlen(cmdline) : 0) +
89 FDT_EXTRA_SPACE;
90
91 fdt = kvmalloc(fdt_size, GFP_KERNEL);
92 if (!fdt)
93 return NULL;
94
95 ret = fdt_open_into(initial_boot_params, fdt, fdt_size);
96 if (ret < 0) {
97 pr_err("Error %d setting up the new device tree.\n", ret);
98 goto out;
99 }
100
101 /* Remove memory reservation for the current device tree. */
102 ret = fdt_find_and_del_mem_rsv(fdt, __pa(initial_boot_params),
103 fdt_totalsize(initial_boot_params));
104 if (ret == -EINVAL) {
105 pr_err("Error removing memory reservation.\n");
106 goto out;
107 }
108
109 chosen_node = fdt_path_offset(fdt, "/chosen");
110 if (chosen_node == -FDT_ERR_NOTFOUND)
111 chosen_node = fdt_add_subnode(fdt, fdt_path_offset(fdt, "/"),
112 "chosen");
113 if (chosen_node < 0) {
114 ret = chosen_node;
115 goto out;
116 }
117
118 ret = fdt_delprop(fdt, chosen_node, FDT_PROP_KEXEC_ELFHDR);
119 if (ret && ret != -FDT_ERR_NOTFOUND)
120 goto out;
121 ret = fdt_delprop(fdt, chosen_node, FDT_PROP_MEM_RANGE);
122 if (ret && ret != -FDT_ERR_NOTFOUND)
123 goto out;
124
125 /* Did we boot using an initrd? */
126 prop = fdt_getprop(fdt, chosen_node, "linux,initrd-start", NULL);
127 if (prop) {
128 u64 tmp_start, tmp_end, tmp_size;
129
130 tmp_start = fdt64_to_cpu(*((const fdt64_t *) prop));
131
132 prop = fdt_getprop(fdt, chosen_node, "linux,initrd-end", NULL);
133 if (!prop) {
134 ret = -EINVAL;
135 goto out;
136 }
137
138 tmp_end = fdt64_to_cpu(*((const fdt64_t *) prop));
139
140 /*
141 * kexec reserves exact initrd size, while firmware may
142 * reserve a multiple of PAGE_SIZE, so check for both.
143 */
144 tmp_size = tmp_end - tmp_start;
145 ret = fdt_find_and_del_mem_rsv(fdt, tmp_start, tmp_size);
146 if (ret == -ENOENT)
147 ret = fdt_find_and_del_mem_rsv(fdt, tmp_start,
148 round_up(tmp_size, PAGE_SIZE));
149 if (ret == -EINVAL)
150 goto out;
151 }
152
153 /* add initrd-* */
154 if (initrd_load_addr) {
155 ret = fdt_setprop_u64(fdt, chosen_node, FDT_PROP_INITRD_START,
156 initrd_load_addr);
157 if (ret)
158 goto out;
159
160 ret = fdt_setprop_u64(fdt, chosen_node, FDT_PROP_INITRD_END,
161 initrd_load_addr + initrd_len);
162 if (ret)
163 goto out;
164
165 ret = fdt_add_mem_rsv(fdt, initrd_load_addr, initrd_len);
166 if (ret)
167 goto out;
168
169 } else {
170 ret = fdt_delprop(fdt, chosen_node, FDT_PROP_INITRD_START);
171 if (ret && (ret != -FDT_ERR_NOTFOUND))
172 goto out;
173
174 ret = fdt_delprop(fdt, chosen_node, FDT_PROP_INITRD_END);
175 if (ret && (ret != -FDT_ERR_NOTFOUND))
176 goto out;
177 }
178
179 if (image->type == KEXEC_TYPE_CRASH) {
180 /* add linux,elfcorehdr */
181 ret = fdt_appendprop_addrrange(fdt, 0, chosen_node,
182 FDT_PROP_KEXEC_ELFHDR,
> 183 image->arch.elf_headers_mem,
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 37859 bytes --]
^ permalink raw reply
* Re: Fwd: Re: [PATCH v17 02/10] of: Add a common kexec FDT setup function
From: Lakshmi Ramasubramanian @ 2021-02-11 17:47 UTC (permalink / raw)
To: Rob Herring, Thiago Jung Bauermann, Mimi Zohar, linux-integrity,
linux-arm-kernel, linux-integrity, linuxppc-dev
In-Reply-To: <40fd1869-dcb4-36ae-e997-b8486dd4846c@linux.microsoft.com>
On 2/11/21 9:42 AM, Lakshmi Ramasubramanian wrote:
> Hi Rob,
>
> [PATCH] powerpc: Rename kexec elfcorehdr_addr to elf_headers_mem
>
> This change causes build problem for x86_64 architecture (please see the
> mail from kernel test bot below) since arch/x86/include/asm/kexec.h uses
> "elf_load_addr" for the ELF header buffer address and not
> "elf_headers_mem".
>
> struct kimage_arch {
> ...
>
> /* Core ELF header buffer */
> void *elf_headers;
> unsigned long elf_headers_sz;
> unsigned long elf_load_addr;
> };
>
> I am thinking of limiting of_kexec_alloc_and_setup_fdt() to ARM64 and
> PPC64 since they are the only ones using this function now.
>
> #if defined(CONFIG_ARM64) && defined(CONFIG_PPC64)
Sorry - I meant to say
#if defined(CONFIG_ARM64) || defined(CONFIG_PPC64)
> void *of_kexec_alloc_and_setup_fdt(const struct kimage *image,
> unsigned long initrd_load_addr,
> unsigned long initrd_len,
> const char *cmdline)
> {
> ...
> }
> #endif /* defined(CONFIG_ARM64) && defined(CONFIG_PPC64) */
>
> Please let me know if you have any concerns.
>
> thanks,
> -lakshmi
>
> -------- Forwarded Message --------
> Subject: Re: [PATCH v17 02/10] of: Add a common kexec FDT setup function
> Date: Fri, 12 Feb 2021 00:50:20 +0800
> From: kernel test robot <lkp@intel.com>
> To: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
> CC: kbuild-all@lists.01.org
>
> Hi Lakshmi,
>
> I love your patch! Yet something to improve:
>
> [auto build test ERROR on integrity/next-integrity]
> [also build test ERROR on v5.11-rc7 next-20210211]
> [cannot apply to powerpc/next robh/for-next arm64/for-next/core]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url:
> https://github.com/0day-ci/linux/commits/Lakshmi-Ramasubramanian/Carry-forward-IMA-measurement-log-on-kexec-on-ARM64/20210211-071924
>
> base:
> https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git next-integrity
>
> config: x86_64-randconfig-m001-20210211 (attached as .config)
> compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
> reproduce (this is a W=1 build):
> #
> https://github.com/0day-ci/linux/commit/12ae86067d115b84092353109e8798693d102f0d
>
> git remote add linux-review https://github.com/0day-ci/linux
> git fetch --no-tags linux-review
> Lakshmi-Ramasubramanian/Carry-forward-IMA-measurement-log-on-kexec-on-ARM64/20210211-071924
>
> git checkout 12ae86067d115b84092353109e8798693d102f0d
> # save the attached .config to linux build tree
> make W=1 ARCH=x86_64
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All errors (new ones prefixed by >>):
>
> drivers/of/kexec.c: In function 'of_kexec_alloc_and_setup_fdt':
>>> drivers/of/kexec.c:183:17: error: 'const struct kimage_arch' has no
>>> member named 'elf_headers_mem'; did you mean 'elf_headers_sz'?
> 183 | image->arch.elf_headers_mem,
> | ^~~~~~~~~~~~~~~
> | elf_headers_sz
> drivers/of/kexec.c:192:42: error: 'const struct kimage_arch' has no
> member named 'elf_headers_mem'; did you mean 'elf_headers_sz'?
> 192 | ret = fdt_add_mem_rsv(fdt, image->arch.elf_headers_mem,
> | ^~~~~~~~~~~~~~~
> | elf_headers_sz
>
>
> vim +183 drivers/of/kexec.c
>
> 65
> 66 /*
> 67 * of_kexec_alloc_and_setup_fdt - Alloc and setup a new
> Flattened Device Tree
> 68 *
> 69 * @image: kexec image being loaded.
> 70 * @initrd_load_addr: Address where the next initrd will
> be loaded.
> 71 * @initrd_len: Size of the next initrd, or 0 if there
> will be none.
> 72 * @cmdline: Command line for the next kernel, or NULL
> if there will
> 73 * be none.
> 74 *
> 75 * Return: fdt on success, or NULL errno on error.
> 76 */
> 77 void *of_kexec_alloc_and_setup_fdt(const struct kimage *image,
> 78 unsigned long initrd_load_addr,
> 79 unsigned long initrd_len,
> 80 const char *cmdline)
> 81 {
> 82 void *fdt;
> 83 int ret, chosen_node;
> 84 const void *prop;
> 85 unsigned long fdt_size;
> 86
> 87 fdt_size = fdt_totalsize(initial_boot_params) +
> 88 (cmdline ? strlen(cmdline) : 0) +
> 89 FDT_EXTRA_SPACE;
> 90
> 91 fdt = kvmalloc(fdt_size, GFP_KERNEL);
> 92 if (!fdt)
> 93 return NULL;
> 94
> 95 ret = fdt_open_into(initial_boot_params, fdt, fdt_size);
> 96 if (ret < 0) {
> 97 pr_err("Error %d setting up the new device tree.\n",
> ret);
> 98 goto out;
> 99 }
> 100
> 101 /* Remove memory reservation for the current device tree. */
> 102 ret = fdt_find_and_del_mem_rsv(fdt,
> __pa(initial_boot_params),
> 103 fdt_totalsize(initial_boot_params));
> 104 if (ret == -EINVAL) {
> 105 pr_err("Error removing memory reservation.\n");
> 106 goto out;
> 107 }
> 108
> 109 chosen_node = fdt_path_offset(fdt, "/chosen");
> 110 if (chosen_node == -FDT_ERR_NOTFOUND)
> 111 chosen_node = fdt_add_subnode(fdt,
> fdt_path_offset(fdt, "/"),
> 112 "chosen");
> 113 if (chosen_node < 0) {
> 114 ret = chosen_node;
> 115 goto out;
> 116 }
> 117
> 118 ret = fdt_delprop(fdt, chosen_node, FDT_PROP_KEXEC_ELFHDR);
> 119 if (ret && ret != -FDT_ERR_NOTFOUND)
> 120 goto out;
> 121 ret = fdt_delprop(fdt, chosen_node, FDT_PROP_MEM_RANGE);
> 122 if (ret && ret != -FDT_ERR_NOTFOUND)
> 123 goto out;
> 124
> 125 /* Did we boot using an initrd? */
> 126 prop = fdt_getprop(fdt, chosen_node,
> "linux,initrd-start", NULL);
> 127 if (prop) {
> 128 u64 tmp_start, tmp_end, tmp_size;
> 129
> 130 tmp_start = fdt64_to_cpu(*((const fdt64_t *) prop));
> 131
> 132 prop = fdt_getprop(fdt, chosen_node,
> "linux,initrd-end", NULL);
> 133 if (!prop) {
> 134 ret = -EINVAL;
> 135 goto out;
> 136 }
> 137
> 138 tmp_end = fdt64_to_cpu(*((const fdt64_t *) prop));
> 139
> 140 /*
> 141 * kexec reserves exact initrd size, while firmware may
> 142 * reserve a multiple of PAGE_SIZE, so check for both.
> 143 */
> 144 tmp_size = tmp_end - tmp_start;
> 145 ret = fdt_find_and_del_mem_rsv(fdt, tmp_start,
> tmp_size);
> 146 if (ret == -ENOENT)
> 147 ret = fdt_find_and_del_mem_rsv(fdt, tmp_start,
> 148 round_up(tmp_size, PAGE_SIZE));
> 149 if (ret == -EINVAL)
> 150 goto out;
> 151 }
> 152
> 153 /* add initrd-* */
> 154 if (initrd_load_addr) {
> 155 ret = fdt_setprop_u64(fdt, chosen_node,
> FDT_PROP_INITRD_START,
> 156 initrd_load_addr);
> 157 if (ret)
> 158 goto out;
> 159
> 160 ret = fdt_setprop_u64(fdt, chosen_node,
> FDT_PROP_INITRD_END,
> 161 initrd_load_addr + initrd_len);
> 162 if (ret)
> 163 goto out;
> 164
> 165 ret = fdt_add_mem_rsv(fdt, initrd_load_addr,
> initrd_len);
> 166 if (ret)
> 167 goto out;
> 168
> 169 } else {
> 170 ret = fdt_delprop(fdt, chosen_node,
> FDT_PROP_INITRD_START);
> 171 if (ret && (ret != -FDT_ERR_NOTFOUND))
> 172 goto out;
> 173
> 174 ret = fdt_delprop(fdt, chosen_node,
> FDT_PROP_INITRD_END);
> 175 if (ret && (ret != -FDT_ERR_NOTFOUND))
> 176 goto out;
> 177 }
> 178
> 179 if (image->type == KEXEC_TYPE_CRASH) {
> 180 /* add linux,elfcorehdr */
> 181 ret = fdt_appendprop_addrrange(fdt, 0, chosen_node,
> 182 FDT_PROP_KEXEC_ELFHDR,
> > 183 image->arch.elf_headers_mem,
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
>
^ permalink raw reply
* [PATCH] powerpc/pseries: extract host bridge from pci_bus prior to bus removal
From: Tyrel Datwyler @ 2021-02-11 18:24 UTC (permalink / raw)
To: mpe; +Cc: linuxppc-dev, linux-kernel, Tyrel Datwyler
The pci_bus->bridge reference may no longer be valid after
pci_bus_remove() resulting in passing a bad value to device_unregister()
for the associated bridge device.
Store the host_bridge reference in a separate variable prior to
pci_bus_remove().
Fixes: 7340056567e3 ("powerpc/pci: Reorder pci bus/bridge unregistration during PHB removal")
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
arch/powerpc/platforms/pseries/pci_dlpar.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/pseries/pci_dlpar.c b/arch/powerpc/platforms/pseries/pci_dlpar.c
index f9ae17e8a0f4..a8f9140a24fa 100644
--- a/arch/powerpc/platforms/pseries/pci_dlpar.c
+++ b/arch/powerpc/platforms/pseries/pci_dlpar.c
@@ -50,6 +50,7 @@ EXPORT_SYMBOL_GPL(init_phb_dynamic);
int remove_phb_dynamic(struct pci_controller *phb)
{
struct pci_bus *b = phb->bus;
+ struct pci_host_bridge *host_bridge = to_pci_host_bridge(b->bridge);
struct resource *res;
int rc, i;
@@ -76,7 +77,8 @@ int remove_phb_dynamic(struct pci_controller *phb)
/* Remove the PCI bus and unregister the bridge device from sysfs */
phb->bus = NULL;
pci_remove_bus(b);
- device_unregister(b->bridge);
+ host_bridge->bus = NULL;
+ device_unregister(&host_bridge->dev);
/* Now release the IO resource */
if (res->flags & IORESOURCE_IO)
--
2.27.0
^ permalink raw reply related
* [PATCH 4/4] ibmvfc: store return code of H_FREE_SUB_CRQ during cleanup
From: Tyrel Datwyler @ 2021-02-11 18:57 UTC (permalink / raw)
To: james.bottomley
Cc: Tyrel Datwyler, martin.petersen, linux-scsi, linux-kernel, brking,
linuxppc-dev
In-Reply-To: <20210211185742.50143-1-tyreld@linux.ibm.com>
The H_FREE_SUB_CRQ hypercall can return a retry delay return code that
indicates the call needs to be retried after a specific amount of time
delay. The error path to free a sub-CRQ in case of a failure during
channel registration fails to capture the return code of H_FREE_SUB_CRQ
which will result in the delay loop being skipped in the case of a retry
delay return code.
Store the return code result of the H_FREE_SUB_CRQ call such that the
return code check in the delay loop evaluates a meaningful value.
Fixes: 9288d35d70b5 ("ibmvfc: map/request irq and register Sub-CRQ interrupt handler")
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index ba6fcf9cbc57..23b803ac4a13 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -5670,7 +5670,7 @@ static int ibmvfc_register_scsi_channel(struct ibmvfc_host *vhost,
irq_failed:
do {
- plpar_hcall_norets(H_FREE_SUB_CRQ, vdev->unit_address, scrq->cookie);
+ rc = plpar_hcall_norets(H_FREE_SUB_CRQ, vdev->unit_address, scrq->cookie);
} while (rc == H_BUSY || H_IS_LONG_BUSY(rc));
reg_failed:
ibmvfc_free_queue(vhost, scrq);
--
2.27.0
^ permalink raw reply related
* [PATCH 1/4] ibmvfc: simplify handling of sub-CRQ initialization
From: Tyrel Datwyler @ 2021-02-11 18:57 UTC (permalink / raw)
To: james.bottomley
Cc: Tyrel Datwyler, martin.petersen, linux-scsi, linux-kernel, brking,
linuxppc-dev
In-Reply-To: <20210211185742.50143-1-tyreld@linux.ibm.com>
If ibmvfc_init_sub_crqs() fails ibmvfc_probe() simply parrots
registration failure reported elsewhere, and futher
vhost->scsi_scrq.scrq == NULL is indication enough to the driver that it
has no sub-CRQs available. The mq_enabled check can also be moved into
ibmvfc_init_sub_crqs() such that each caller doesn't have to gate the
call with a mq_enabled check. Finally, in the case of sub-CRQ setup
failure setting do_enquiry can be turned off to putting the driver into
single queue fallback mode.
The aforementioned changes also simplify the next patch in the series
that fixes a hard reset issue, by tying a sub-CRQ setup failure and
do_enquiry logic into ibmvfc_init_sub_crqs().
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc.c | 21 ++++++++++-----------
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index 7097028d4cb6..384960036f8b 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -5705,17 +5705,21 @@ static void ibmvfc_deregister_scsi_channel(struct ibmvfc_host *vhost, int index)
LEAVE;
}
-static int ibmvfc_init_sub_crqs(struct ibmvfc_host *vhost)
+static void ibmvfc_init_sub_crqs(struct ibmvfc_host *vhost)
{
int i, j;
ENTER;
+ if (!vhost->mq_enabled)
+ return;
vhost->scsi_scrqs.scrqs = kcalloc(nr_scsi_hw_queues,
sizeof(*vhost->scsi_scrqs.scrqs),
GFP_KERNEL);
- if (!vhost->scsi_scrqs.scrqs)
- return -1;
+ if (!vhost->scsi_scrqs.scrqs) {
+ vhost->do_enquiry = 0;
+ return;
+ }
for (i = 0; i < nr_scsi_hw_queues; i++) {
if (ibmvfc_register_scsi_channel(vhost, i)) {
@@ -5724,13 +5728,12 @@ static int ibmvfc_init_sub_crqs(struct ibmvfc_host *vhost)
kfree(vhost->scsi_scrqs.scrqs);
vhost->scsi_scrqs.scrqs = NULL;
vhost->scsi_scrqs.active_queues = 0;
- LEAVE;
- return -1;
+ vhost->do_enquiry = 0;
+ break;
}
}
LEAVE;
- return 0;
}
static void ibmvfc_release_sub_crqs(struct ibmvfc_host *vhost)
@@ -5997,11 +6000,7 @@ static int ibmvfc_probe(struct vio_dev *vdev, const struct vio_device_id *id)
goto remove_shost;
}
- if (vhost->mq_enabled) {
- rc = ibmvfc_init_sub_crqs(vhost);
- if (rc)
- dev_warn(dev, "Failed to allocate Sub-CRQs. rc=%d\n", rc);
- }
+ ibmvfc_init_sub_crqs(vhost);
if (shost_to_fc_host(shost)->rqst_q)
blk_queue_max_segments(shost_to_fc_host(shost)->rqst_q, 1);
--
2.27.0
^ permalink raw reply related
* [PATCH 0/4] ibmvfc: hard reset fixes
From: Tyrel Datwyler @ 2021-02-11 18:57 UTC (permalink / raw)
To: james.bottomley
Cc: Tyrel Datwyler, martin.petersen, linux-scsi, linux-kernel, brking,
linuxppc-dev
This series contains a minor simplification of ibmvfc_init_sub_crqs() followed
by a couple fixes for sub-CRQ handling which effect hard reset of the
client/host adapter CRQ pair.
^ permalink raw reply
* [PATCH 2/4] ibmvfc: fix invalid sub-CRQ handles after hard reset
From: Tyrel Datwyler @ 2021-02-11 18:57 UTC (permalink / raw)
To: james.bottomley
Cc: Tyrel Datwyler, martin.petersen, linux-scsi, linux-kernel, brking,
linuxppc-dev
In-Reply-To: <20210211185742.50143-1-tyreld@linux.ibm.com>
A hard reset results in a complete transport disconnect such that the
CRQ connection with the partner VIOS is broken. This has the side effect
of also invalidating the associated sub-CRQs. The current code assumes
that the sub-CRQs are perserved resulting in a protocol violation after
trying to reconnect them with the VIOS. This introduces an infinite loop
such that the VIOS forces a disconnect after each subsequent attempt to
re-register with invalid handles.
Avoid the aforementioned issue by releasing the sub-CRQs prior to CRQ
disconnect, and driving a reinitialization of the sub-CRQs once a new
CRQ is registered with the hypervisor.
fixes: faacf8c5f1d5 ("ibmvfc: add alloc/dealloc routines for SCSI Sub-CRQ Channels")
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc.c | 21 +++++++++------------
1 file changed, 9 insertions(+), 12 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index 384960036f8b..3dd20f383453 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -158,6 +158,9 @@ static void ibmvfc_npiv_logout(struct ibmvfc_host *);
static void ibmvfc_tgt_implicit_logout_and_del(struct ibmvfc_target *);
static void ibmvfc_tgt_move_login(struct ibmvfc_target *);
+static void ibmvfc_release_sub_crqs(struct ibmvfc_host *);
+static void ibmvfc_init_sub_crqs(struct ibmvfc_host *);
+
static const char *unknown_error = "unknown error";
static long h_reg_sub_crq(unsigned long unit_address, unsigned long ioba,
@@ -926,8 +929,8 @@ static int ibmvfc_reset_crq(struct ibmvfc_host *vhost)
unsigned long flags;
struct vio_dev *vdev = to_vio_dev(vhost->dev);
struct ibmvfc_queue *crq = &vhost->crq;
- struct ibmvfc_queue *scrq;
- int i;
+
+ ibmvfc_release_sub_crqs(vhost);
/* Close the CRQ */
do {
@@ -947,16 +950,6 @@ static int ibmvfc_reset_crq(struct ibmvfc_host *vhost)
memset(crq->msgs.crq, 0, PAGE_SIZE);
crq->cur = 0;
- if (vhost->scsi_scrqs.scrqs) {
- for (i = 0; i < nr_scsi_hw_queues; i++) {
- scrq = &vhost->scsi_scrqs.scrqs[i];
- spin_lock(scrq->q_lock);
- memset(scrq->msgs.scrq, 0, PAGE_SIZE);
- scrq->cur = 0;
- spin_unlock(scrq->q_lock);
- }
- }
-
/* And re-open it again */
rc = plpar_hcall_norets(H_REG_CRQ, vdev->unit_address,
crq->msg_token, PAGE_SIZE);
@@ -966,6 +959,9 @@ static int ibmvfc_reset_crq(struct ibmvfc_host *vhost)
dev_warn(vhost->dev, "Partner adapter not ready\n");
else if (rc != 0)
dev_warn(vhost->dev, "Couldn't register crq (rc=%d)\n", rc);
+
+ ibmvfc_init_sub_crqs(vhost);
+
spin_unlock(vhost->crq.q_lock);
spin_unlock_irqrestore(vhost->host->host_lock, flags);
@@ -5692,6 +5688,7 @@ static void ibmvfc_deregister_scsi_channel(struct ibmvfc_host *vhost, int index)
free_irq(scrq->irq, scrq);
irq_dispose_mapping(scrq->irq);
+ scrq->irq = 0;
do {
rc = plpar_hcall_norets(H_FREE_SUB_CRQ, vdev->unit_address,
--
2.27.0
^ permalink raw reply related
* [PATCH 3/4] ibmvfc: treat H_CLOSED as success during sub-CRQ registration
From: Tyrel Datwyler @ 2021-02-11 18:57 UTC (permalink / raw)
To: james.bottomley
Cc: Tyrel Datwyler, martin.petersen, linux-scsi, linux-kernel, brking,
linuxppc-dev
In-Reply-To: <20210211185742.50143-1-tyreld@linux.ibm.com>
A non-zero return code for H_REG_SUB_CRQ is currently treated as a
failure resulting in failing sub-CRQ setup. The case of H_CLOSED should
not be treated as a failure. This return code translates to a successful
sub-CRQ registration by the hypervisor, and is meant to communicate back
that there is currently no partner VIOS CRQ connection established as of
yet. This is a common occurrence during a disconnect where the client
adapter can possibly come back up prior to the partner adapter.
For non-zero return code from H_REG_SUB_CRQ treat a H_CLOSED as success
so that sub-CRQs are successfully setup.
Fixes: faacf8c5f1d5 ("ibmvfc: add alloc/dealloc routines for SCSI Sub-CRQ Channels")
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index 3dd20f383453..ba6fcf9cbc57 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -5636,7 +5636,8 @@ static int ibmvfc_register_scsi_channel(struct ibmvfc_host *vhost,
rc = h_reg_sub_crq(vdev->unit_address, scrq->msg_token, PAGE_SIZE,
&scrq->cookie, &scrq->hw_irq);
- if (rc) {
+ /* H_CLOSED indicates successful register, but no CRQ partner */
+ if (rc && rc != H_CLOSED) {
dev_warn(dev, "Error registering sub-crq: %d\n", rc);
if (rc == H_PARAMETER)
dev_warn_once(dev, "Firmware may not support MQ\n");
--
2.27.0
^ permalink raw reply related
* [PATCH 1/2] of: Remove of_dev_{get,put}()
From: Rob Herring @ 2021-02-11 22:25 UTC (permalink / raw)
To: Michael Ellerman, Greg Kroah-Hartman, David S. Miller,
Jakub Kicinski, Frank Rowand, devicetree
Cc: Felipe Balbi, Michal Marek, Rafael J. Wysocki, netdev, linux-usb,
Nicolas Palix, Patrice Chotard, linux-kernel, Julia Lawall,
Gilles Muller, Paul Mackerras, linuxppc-dev, cocci,
linux-arm-kernel
In-Reply-To: <20210211222526.1318236-1-robh@kernel.org>
of_dev_get() and of_dev_put are just wrappers for get_device()/put_device()
on a platform_device. There's also already platform_device_{get,put}()
wrappers for this purpose. Let's update the few users and remove
of_dev_{get,put}().
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Frank Rowand <frowand.list@gmail.com>
Cc: Patrice Chotard <patrice.chotard@st.com>
Cc: Felipe Balbi <balbi@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Julia Lawall <Julia.Lawall@inria.fr>
Cc: Gilles Muller <Gilles.Muller@inria.fr>
Cc: Nicolas Palix <nicolas.palix@imag.fr>
Cc: Michal Marek <michal.lkml@markovi.net>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: netdev@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-usb@vger.kernel.org
Cc: cocci@systeme.lip6.fr
Signed-off-by: Rob Herring <robh@kernel.org>
---
arch/powerpc/platforms/pseries/ibmebus.c | 4 ++--
drivers/net/ethernet/ibm/emac/core.c | 15 ++++++++-------
drivers/of/device.c | 21 ---------------------
drivers/of/platform.c | 4 ++--
drivers/of/unittest.c | 2 +-
drivers/usb/dwc3/dwc3-st.c | 2 +-
include/linux/of_device.h | 3 ---
scripts/coccinelle/free/put_device.cocci | 1 -
8 files changed, 14 insertions(+), 38 deletions(-)
diff --git a/arch/powerpc/platforms/pseries/ibmebus.c b/arch/powerpc/platforms/pseries/ibmebus.c
index 8c6e509f6967..c29328fe94e8 100644
--- a/arch/powerpc/platforms/pseries/ibmebus.c
+++ b/arch/powerpc/platforms/pseries/ibmebus.c
@@ -355,12 +355,12 @@ static int ibmebus_bus_device_probe(struct device *dev)
if (!drv->probe)
return error;
- of_dev_get(of_dev);
+ get_device(dev);
if (of_driver_match_device(dev, dev->driver))
error = drv->probe(of_dev);
if (error)
- of_dev_put(of_dev);
+ put_device(of_dev);
return error;
}
diff --git a/drivers/net/ethernet/ibm/emac/core.c b/drivers/net/ethernet/ibm/emac/core.c
index c00b9097eeea..471be6ec7e8a 100644
--- a/drivers/net/ethernet/ibm/emac/core.c
+++ b/drivers/net/ethernet/ibm/emac/core.c
@@ -38,6 +38,7 @@
#include <linux/of_irq.h>
#include <linux/of_net.h>
#include <linux/of_mdio.h>
+#include <linux/platform_device.h>
#include <linux/slab.h>
#include <asm/processor.h>
@@ -2390,11 +2391,11 @@ static int emac_check_deps(struct emac_instance *dev,
static void emac_put_deps(struct emac_instance *dev)
{
- of_dev_put(dev->mal_dev);
- of_dev_put(dev->zmii_dev);
- of_dev_put(dev->rgmii_dev);
- of_dev_put(dev->mdio_dev);
- of_dev_put(dev->tah_dev);
+ platform_device_put(dev->mal_dev);
+ platform_device_put(dev->zmii_dev);
+ platform_device_put(dev->rgmii_dev);
+ platform_device_put(dev->mdio_dev);
+ platform_device_put(dev->tah_dev);
}
static int emac_of_bus_notify(struct notifier_block *nb, unsigned long action,
@@ -2435,7 +2436,7 @@ static int emac_wait_deps(struct emac_instance *dev)
for (i = 0; i < EMAC_DEP_COUNT; i++) {
of_node_put(deps[i].node);
if (err)
- of_dev_put(deps[i].ofdev);
+ platform_device_put(deps[i].ofdev);
}
if (err == 0) {
dev->mal_dev = deps[EMAC_DEP_MAL_IDX].ofdev;
@@ -2444,7 +2445,7 @@ static int emac_wait_deps(struct emac_instance *dev)
dev->tah_dev = deps[EMAC_DEP_TAH_IDX].ofdev;
dev->mdio_dev = deps[EMAC_DEP_MDIO_IDX].ofdev;
}
- of_dev_put(deps[EMAC_DEP_PREV_IDX].ofdev);
+ platform_device_put(deps[EMAC_DEP_PREV_IDX].ofdev);
return err;
}
diff --git a/drivers/of/device.c b/drivers/of/device.c
index aedfaaafd3e7..9a748855b39d 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -33,27 +33,6 @@ const struct of_device_id *of_match_device(const struct of_device_id *matches,
}
EXPORT_SYMBOL(of_match_device);
-struct platform_device *of_dev_get(struct platform_device *dev)
-{
- struct device *tmp;
-
- if (!dev)
- return NULL;
- tmp = get_device(&dev->dev);
- if (tmp)
- return to_platform_device(tmp);
- else
- return NULL;
-}
-EXPORT_SYMBOL(of_dev_get);
-
-void of_dev_put(struct platform_device *dev)
-{
- if (dev)
- put_device(&dev->dev);
-}
-EXPORT_SYMBOL(of_dev_put);
-
int of_device_add(struct platform_device *ofdev)
{
BUG_ON(ofdev->dev.of_node == NULL);
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 79bd5f5a1bf1..020bf860c72c 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -687,7 +687,7 @@ static int of_platform_notify(struct notifier_block *nb,
pdev_parent = of_find_device_by_node(rd->dn->parent);
pdev = of_platform_device_create(rd->dn, NULL,
pdev_parent ? &pdev_parent->dev : NULL);
- of_dev_put(pdev_parent);
+ platform_device_put(pdev_parent);
if (pdev == NULL) {
pr_err("%s: failed to create for '%pOF'\n",
@@ -712,7 +712,7 @@ static int of_platform_notify(struct notifier_block *nb,
of_platform_device_destroy(&pdev->dev, &children_left);
/* and put the reference of the find */
- of_dev_put(pdev);
+ platform_device_put(pdev);
break;
}
diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index eb51bc147440..eb100627c186 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -1286,7 +1286,7 @@ static void __init of_unittest_platform_populate(void)
unittest(pdev,
"Could not create device for node '%pOFn'\n",
grandchild);
- of_dev_put(pdev);
+ platform_device_put(pdev);
}
}
diff --git a/drivers/usb/dwc3/dwc3-st.c b/drivers/usb/dwc3/dwc3-st.c
index e733be840545..b06b7092b1a2 100644
--- a/drivers/usb/dwc3/dwc3-st.c
+++ b/drivers/usb/dwc3/dwc3-st.c
@@ -274,7 +274,7 @@ static int st_dwc3_probe(struct platform_device *pdev)
dwc3_data->dr_mode = usb_get_dr_mode(&child_pdev->dev);
of_node_put(child);
- of_dev_put(child_pdev);
+ platform_device_put(child_pdev);
/*
* Configure the USB port as device or host according to the static
diff --git a/include/linux/of_device.h b/include/linux/of_device.h
index 937f32f6aecb..d7a407dfeecb 100644
--- a/include/linux/of_device.h
+++ b/include/linux/of_device.h
@@ -26,9 +26,6 @@ static inline int of_driver_match_device(struct device *dev,
return of_match_device(drv->of_match_table, dev) != NULL;
}
-extern struct platform_device *of_dev_get(struct platform_device *dev);
-extern void of_dev_put(struct platform_device *dev);
-
extern int of_device_add(struct platform_device *pdev);
extern int of_device_register(struct platform_device *ofdev);
extern void of_device_unregister(struct platform_device *ofdev);
diff --git a/scripts/coccinelle/free/put_device.cocci b/scripts/coccinelle/free/put_device.cocci
index 120921366e84..f09f1e79bfa6 100644
--- a/scripts/coccinelle/free/put_device.cocci
+++ b/scripts/coccinelle/free/put_device.cocci
@@ -21,7 +21,6 @@ id = of_find_device_by_node@p1(x)
if (id == NULL || ...) { ... return ...; }
... when != put_device(&id->dev)
when != platform_device_put(id)
- when != of_dev_put(id)
when != if (id) { ... put_device(&id->dev) ... }
when != e1 = (T)id
when != e1 = (T)(&id->dev)
--
2.27.0
^ permalink raw reply related
* [PATCH 0/2] of: of_device.h cleanups
From: Rob Herring @ 2021-02-11 22:25 UTC (permalink / raw)
To: Michael Ellerman, Greg Kroah-Hartman, David S. Miller,
Jakub Kicinski, Frank Rowand, devicetree
Cc: Felipe Balbi, Michal Marek, Rafael J. Wysocki, netdev, linux-usb,
Nicolas Palix, Patrice Chotard, linux-kernel, Julia Lawall,
Gilles Muller, Paul Mackerras, linuxppc-dev, cocci,
linux-arm-kernel
This is a couple of cleanups for of_device.h. They fell out from my
attempt at decoupling of_device.h and of_platform.h which is a mess
and I haven't finished, but there's no reason to wait on these.
Rob
Rob Herring (2):
of: Remove of_dev_{get,put}()
driver core: platform: Drop of_device_node_put() wrapper
arch/powerpc/platforms/pseries/ibmebus.c | 4 ++--
drivers/base/platform.c | 2 +-
drivers/net/ethernet/ibm/emac/core.c | 15 ++++++++-------
drivers/of/device.c | 21 ---------------------
drivers/of/platform.c | 4 ++--
drivers/of/unittest.c | 2 +-
drivers/usb/dwc3/dwc3-st.c | 2 +-
include/linux/of_device.h | 10 ----------
scripts/coccinelle/free/put_device.cocci | 1 -
9 files changed, 15 insertions(+), 46 deletions(-)
--
2.27.0
^ permalink raw reply
* [PATCH 2/2] driver core: platform: Drop of_device_node_put() wrapper
From: Rob Herring @ 2021-02-11 22:25 UTC (permalink / raw)
To: Michael Ellerman, Greg Kroah-Hartman, David S. Miller,
Jakub Kicinski, Frank Rowand, devicetree
Cc: Felipe Balbi, Michal Marek, Rafael J. Wysocki, netdev, linux-usb,
Nicolas Palix, Patrice Chotard, linux-kernel, Julia Lawall,
Gilles Muller, Paul Mackerras, linuxppc-dev, cocci,
linux-arm-kernel
In-Reply-To: <20210211222526.1318236-1-robh@kernel.org>
of_device_node_put() is just a wrapper for of_node_put(). The platform
driver core is already polluted with of_node pointers and the only 'get'
already uses of_node_get() (though typically the get would happen in
of_device_alloc()).
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: Frank Rowand <frowand.list@gmail.com>
Signed-off-by: Rob Herring <robh@kernel.org>
---
drivers/base/platform.c | 2 +-
include/linux/of_device.h | 7 -------
2 files changed, 1 insertion(+), 8 deletions(-)
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 95fd1549f87d..c31bc9e92dd1 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -571,7 +571,7 @@ static void platform_device_release(struct device *dev)
struct platform_object *pa = container_of(dev, struct platform_object,
pdev.dev);
- of_device_node_put(&pa->pdev.dev);
+ of_node_put(&pa->pdev.dev->of_node);
kfree(pa->pdev.dev.platform_data);
kfree(pa->pdev.mfd_cell);
kfree(pa->pdev.resource);
diff --git a/include/linux/of_device.h b/include/linux/of_device.h
index d7a407dfeecb..1d7992a02e36 100644
--- a/include/linux/of_device.h
+++ b/include/linux/of_device.h
@@ -38,11 +38,6 @@ extern int of_device_request_module(struct device *dev);
extern void of_device_uevent(struct device *dev, struct kobj_uevent_env *env);
extern int of_device_uevent_modalias(struct device *dev, struct kobj_uevent_env *env);
-static inline void of_device_node_put(struct device *dev)
-{
- of_node_put(dev->of_node);
-}
-
static inline struct device_node *of_cpu_device_node_get(int cpu)
{
struct device *cpu_dev;
@@ -94,8 +89,6 @@ static inline int of_device_uevent_modalias(struct device *dev,
return -ENODEV;
}
-static inline void of_device_node_put(struct device *dev) { }
-
static inline const struct of_device_id *of_match_device(
const struct of_device_id *matches, const struct device *dev)
{
--
2.27.0
^ permalink raw reply related
* Re: [PATCH] powerpc/bug: Remove specific powerpc BUG_ON()
From: Nicholas Piggin @ 2021-02-11 22:47 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linux-kernel, Paul Mackerras, linuxppc-dev
In-Reply-To: <20210211115059.GB28121@gate.crashing.org>
Excerpts from Segher Boessenkool's message of February 11, 2021 9:50 pm:
> On Thu, Feb 11, 2021 at 08:04:55PM +1000, Nicholas Piggin wrote:
>> It would be nice if we could have a __builtin_trap_if that gcc would use
>> conditional traps with, (and which never assumes following code is
>> unreachable even for constant true, so we can use it with WARN and put
>> explicit unreachable for BUG).
>
> It automatically does that with just __builtin_trap, see my other mail :-)
If that is generated without branches (or at least with no more
branches than existing asm implementation), then it could be usable
without trashing CFAR.
Unfortunately I don't think we will be parsing the dwarf information
to get line numbers from it any time soon, so not a drop in replacement
but maybe one day someone would find a solution.
Thanks,
Nick
^ permalink raw reply
* [GIT PULL] Please pull powerpc/linux.git powerpc-5.11-8 tag
From: Michael Ellerman @ 2021-02-11 23:15 UTC (permalink / raw)
To: Linus Torvalds; +Cc: aneesh.kumar, linuxppc-dev, linux-kernel
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
Hi Linus,
Please pull one final powerpc fix for 5.11:
The following changes since commit 24321ac668e452a4942598533d267805f291fdc9:
powerpc/64/signal: Fix regression in __kernel_sigtramp_rt64() semantics (2021-02-02 22:14:41 +1100)
are available in the git repository at:
https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git tags/powerpc-5.11-8
for you to fetch changes up to 8c511eff1827239f24ded212b1bcda7ca5b16203:
powerpc/kuap: Allow kernel thread to access userspace after kthread_use_mm (2021-02-06 23:13:04 +1100)
- ------------------------------------------------------------------
powerpc fixes for 5.11 #8
One fix for a regression seen in io_uring, introduced by our support for KUAP
(Kernel User Access Prevention) with the Hash MMU.
Thanks to: Aneesh Kumar K.V, Zorro Lang.
- ------------------------------------------------------------------
Aneesh Kumar K.V (1):
powerpc/kuap: Allow kernel thread to access userspace after kthread_use_mm
arch/powerpc/include/asm/book3s/64/kup.h | 16 +++++++++++-----
arch/powerpc/include/asm/book3s/64/pkeys.h | 4 ----
arch/powerpc/mm/book3s64/pkeys.c | 1 +
3 files changed, 12 insertions(+), 9 deletions(-)
-----BEGIN PGP SIGNATURE-----
iQIzBAEBCAAdFiEEJFGtCPCthwEv2Y/bUevqPMjhpYAFAmAlumMACgkQUevqPMjh
pYBb9BAAqpyhvEjDiyP0xeHUTQ9UsNMnNUW7Hp03jJYH+EKWC9nT7kb3TY1eNyk5
KyMBOtXiQsggzWDE31bR32BpH/JChSeRPaCO4HHdDS9t+3ZIo16RTFksdiIhCN6m
nI4WhnfrdZszstMRsKWzfoLRVDGyNi0hyyQaMS3ypuKvRmozZk6u9K/YNXMa97Wf
ihhB0lYRdfMNgxMm66uaqEtzYt3Z4dRj9Y24LQirJnp32xK+sNgoleHl4gvvKG3m
r7CogqHlcExbkD3dl/PPe/SVEesfXpmTrQPCvJmi0qWm9NzkduQWrSEkUkUp1YQD
T0pBHnCxtI7GAAQpCphBA3gjrz03Og4/RAXmfESgI0JHyh7Vihx91XOwuonuJndn
5ThY2D9+nkZ2vnlis2/AoLx6ClbNgZysr0iAOsRyd2SYR9Er2CcPZ4OuNHXWTHlz
G4SmVYiZj9gnSrzqlEGIqBOVWdykV+x+xkLLQx86HUAI+7f1mFV1+dJ4E5NLGKzS
jB+XwwG2y6q6SnJt2iiybqDu9K1lPWnKFLNeb4at7GfpJ4riKNcRSYrkY9xYxmkR
kgKyXfW+rNt1RcIoy65kNa3hSnXi4p9wc5Lph7joS7zTkZIKR2E3QUJjGQao85Qa
rNxccSz3X7ZAL9OEJP/4nE72Zf3VXkzuKbB79D3dhFJ8SJmbPqQ=
=GDL5
-----END PGP SIGNATURE-----
^ 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