Re: [PATCH v2 01/13] powerpc: Move 'struct ppc64_opd_entry' back into asm/elf.h
Excerpts from Christophe Leroy's message of October 14, 2021 3:49 pm:
> 'struct ppc64_opd_entry' doesn't belong to uapi/asm/elf.h
>
> It was initially in module_64.c and commit 2d291e902791 ("Fix compile
> failure with non modular builds") moved it into asm/elf.h
>
> But it was by mistake added outside of __KERNEL__ section,
> therefore commit c3617f72036c ("UAPI: (Scripted) Disintegrate
> arch/powerpc/include/asm") moved it to uapi/asm/elf.h
>
> Move it back into asm/elf.h, this brings it back in line with
> IA64 and PARISC architectures.
>
> Fixes: 2d291e902791 ("Fix compile failure with non modular builds")
> Reviewed-by: Kees Cook
> Signed-off-by: Christophe Leroy
> ---
> arch/powerpc/include/asm/elf.h | 6 ++
> arch/powerpc/include/uapi/asm/elf.h | 8
> 2 files changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/elf.h b/arch/powerpc/include/asm/elf.h
> index b8425e3cfd81..a4406714c060 100644
> --- a/arch/powerpc/include/asm/elf.h
> +++ b/arch/powerpc/include/asm/elf.h
> @@ -176,4 +176,10 @@ do {
> \
> /* Relocate the kernel image to @final_address */
> void relocate(unsigned long final_address);
>
> +/* There's actually a third entry here, but it's unused */
> +struct ppc64_opd_entry {
> + unsigned long funcaddr;
> + unsigned long r2;
> +};
Reviewed-by: Nicholas Piggin
I wonder if we should add that third entry, just for completeness. And
'r2' isn't a good name should probably be toc. And should it be packed?
At any rate that's not for your series, a cleanup I might think about
for later.
Thanks,
Nick
[powerpc:next] BUILD SUCCESS 8f6aca0e0f26eaaee670cd27896993a45cdc8f9e
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next branch HEAD: 8f6aca0e0f26eaaee670cd27896993a45cdc8f9e powerpc/perf: Fix cycles/instructions as PM_CYC/PM_INST_CMPL in power10 elapsed time: 916m configs tested: 154 configs skipped: 3 The following configs have been built successfully. More configs may be tested in the coming days. gcc tested configs: arm defconfig arm64allyesconfig arm64 defconfig arm allyesconfig arm allmodconfig i386 randconfig-c001-20211014 mips loongson1b_defconfig armmmp2_defconfig mips rb532_defconfig arm exynos_defconfig shedosk7705_defconfig armzeus_defconfig sh se7206_defconfig sh ap325rxa_defconfig powerpcsocrates_defconfig powerpc ebony_defconfig powerpc kmeter1_defconfig s390 zfcpdump_defconfig mipsqi_lb60_defconfig arm colibri_pxa270_defconfig powerpc tqm8560_defconfig armqcom_defconfig arc axs103_smp_defconfig powerpc currituck_defconfig mips decstation_64_defconfig powerpc skiroot_defconfig xtensa virt_defconfig mips bigsur_defconfig arc alldefconfig m68kq40_defconfig shedosk7760_defconfig mipsmalta_qemu_32r6_defconfig powerpc lite5200b_defconfig arm orion5x_defconfig mipsworkpad_defconfig mips loongson2k_defconfig arc haps_hs_smp_defconfig m68k atari_defconfig um defconfig sh rts7751r2d1_defconfig arc defconfig riscvalldefconfig powerpc ppc40x_defconfig m68k sun3_defconfig m68kstmark2_defconfig powerpc tqm8xx_defconfig powerpcadder875_defconfig arcnsim_700_defconfig powerpc tqm8541_defconfig nios2 3c120_defconfig mips fuloong2e_defconfig powerpcwarp_defconfig openriscor1ksim_defconfig arm gemini_defconfig powerpc pasemi_defconfig m68k apollo_defconfig arm imx_v6_v7_defconfig shmigor_defconfig powerpc stx_gp3_defconfig arm colibri_pxa300_defconfig s390 debug_defconfig sh j2_defconfig sh espt_defconfig mipsbcm47xx_defconfig powerpc mpc8560_ads_defconfig armvexpress_defconfig powerpc pmac32_defconfig powerpc mpc8315_rdb_defconfig powerpcge_imp3a_defconfig powerpc sequoia_defconfig m68k hp300_defconfig sh rsk7269_defconfig ia64defconfig powerpc arches_defconfig riscv rv32_defconfig shsh7757lcr_defconfig arm randconfig-c002-20211014 x86_64 randconfig-c001-20211014 ia64 allmodconfig ia64 allyesconfig m68k allmodconfig m68kdefconfig m68k allyesconfig nios2 defconfig nds32 allnoconfig arc allyesconfig nds32 defconfig nios2allyesconfig cskydefconfig alpha defconfig alphaallyesconfig xtensa allyesconfig h8300allyesconfig sh allmodconfig parisc defconfig s390defconfig s390 allyesconfig s390 allmodconfig parisc allyesconfig sparcallyesconfig
Re: [PATCH v2 03/13] powerpc: Remove func_descr_t
Le 15/10/2021 à 00:17, Daniel Axtens a écrit :
Christophe Leroy writes:
'func_descr_t' is redundant with 'struct ppc64_opd_entry'
So, if I understand the overall direction of the series, you're
consolidating powerpc around one single type for function descriptors,
and then you're creating a generic typedef so that generic code can
always do ((func_desc_t)x)->addr to get the address of a function out of
a function descriptor regardless of arch. (And regardless of whether the
arch uses function descriptors or not.)
An architecture not using function descriptors won't do much with
((func_desc_t *)x)->addr. This is just done to allow building stuff
regardless.
I prefer something like
if (have_function_descriptors())
addr = (func_desc_t *)ptr)->addr;
else
addr = ptr;
over
#ifdef HAVE_FUNCTION_DESCRIPTORS
addr = (func_desc_t *)ptr)->addr;
#else
addr = ptr;
#endif
So:
- why pick ppc64_opd_entry over func_descr_t?
Good question. At the begining it was because it was in UAPI headers,
and also because it was the one used in our
dereference_function_descriptor().
But at the end maybe that's not the more logical choice. I need to look
a bit more.
- Why not make our struct just called func_desc_t - why have a
ppc64_opd_entry type or a func_descr_t typedef?
Well ... you usually don't flag a struct name with _t, _t will most of
the time refer to a typedef.
If I want to avoid typedef (I know they are deprecated in kernel coding
stype), it means the name of the struct must be changed in every
architecture and it becomes tricky and it adds more churn in them, which
is what I want to avoid.
At the end we risk to end-up with a messy set of #ifdefs.
Maybe this can be done as a second step, but I would like to minimise
impact in this series and focus on fixing lkdtm.
- Should this patch wait until after you've made the generic
func_desc_t change and move directly to that new interface? (rather
than move from func_descr_t -> ppc64_opd_entry -> ...) Or is there a
particular reason arch specific code should use an arch-specific
struct or named type?
As mentioned in kernel coding style, typedefs reduce readability, see
https://www.kernel.org/doc/html/latest/process/coding-style.html#typedefs
But yes, we could make a step in the direction of a common 'struct
func_desc'. Let's see if I can do that.
Thanks for your comments.
Christophe
Remove it.
Signed-off-by: Christophe Leroy
---
arch/powerpc/include/asm/code-patching.h | 2 +-
arch/powerpc/include/asm/types.h | 6 --
arch/powerpc/kernel/signal_64.c | 8
3 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/include/asm/code-patching.h
b/arch/powerpc/include/asm/code-patching.h
index 4ba834599c4d..f3445188d319 100644
--- a/arch/powerpc/include/asm/code-patching.h
+++ b/arch/powerpc/include/asm/code-patching.h
@@ -110,7 +110,7 @@ static inline unsigned long ppc_function_entry(void *func)
* function's descriptor. The first entry in the descriptor is the
* address of the function text.
*/
- return ((func_descr_t *)func)->entry;
+ return ((struct ppc64_opd_entry *)func)->addr;
#else
return (unsigned long)func;
#endif
diff --git a/arch/powerpc/include/asm/types.h b/arch/powerpc/include/asm/types.h
index f1630c553efe..97da77bc48c9 100644
--- a/arch/powerpc/include/asm/types.h
+++ b/arch/powerpc/include/asm/types.h
@@ -23,12 +23,6 @@
typedef __vector128 vector128;
-typedef struct {
- unsigned long entry;
- unsigned long toc;
- unsigned long env;
-} func_descr_t;
I was a little concerned about going from a 3-element struct to a
2-element struct (as ppc64_opd_entry doesn't have an element for env) -
but we don't seem to take the sizeof this anywhere, nor do we use env
anywhere, nor do we do funky macro stuff with it in the signal handling
code that might implictly use the 3rd element, so I guess this will
work. Still, func_descr_t seems to describe the underlying ABI better
than ppc64_opd_entry...
#endif /* __ASSEMBLY__ */
#endif /* _ASM_POWERPC_TYPES_H */
diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
index 1831bba0582e..63ddbe7b108c 100644
--- a/arch/powerpc/kernel/signal_64.c
+++ b/arch/powerpc/kernel/signal_64.c
@@ -933,11 +933,11 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t
*set,
* descriptor is the entry address of signal and the second
* entry is the TOC value we need to use.
*/
- func_descr_t __user *funct_desc_ptr =
- (func_descr_t __user *) ksig->ka.sa.sa_handler;
+ struct ppc64_opd_entry __user *funct_desc_ptr =
+ (struct ppc64_opd_entry __user *)ksig->ka.sa.sa_handler;
-
Re: [PATCH v2 02/13] powerpc: Rename 'funcaddr' to 'addr' in 'struct ppc64_opd_entry'
Le 14/10/2021 à 23:45, Daniel Axtens a écrit :
Christophe Leroy writes:
There are three architectures with function descriptors, try to
have common names for the address they contain in order to
refactor some functions into generic functions later.
powerpc has 'funcaddr'
ia64 has 'ip'
parisc has 'addr'
Vote for 'addr' and update 'struct ppc64_opd_entry' accordingly.
I would have picked 'funcaddr', but at least 'addr' is better than 'ip'!
And I agree that consistency, and then making things generic is worthwhile.
It's a function descriptor, there is only one address field, I don't
think there is any ambiguïty here, and I prefer modifying the least
impacted architectures.
Changing addr to funcaddr in PARISC would result in the following
changes, on an architecture I know nothing about. It's more changes than
we have on powerpc.
arch/parisc/include/asm/elf.h | 4 ++--
arch/parisc/kernel/kexec.c| 2 +-
arch/parisc/kernel/module.c | 12 ++--
arch/parisc/kernel/process.c | 2 +-
arch/parisc/kernel/signal.c | 4 ++--
5 files changed, 12 insertions(+), 12 deletions(-)
I grepped the latest powerpc/next for uses of 'funcaddr'. There were 5,
your patch changes all 5.
The series passes build tests and this patch has no checkpatch or other
style concerns.
On that basis:
Reviewed-by: Daniel Axtens
Kind regards,
Daniel
Reviewed-by: Kees Cook
Signed-off-by: Christophe Leroy
---
arch/powerpc/include/asm/elf.h | 2 +-
arch/powerpc/include/asm/sections.h | 2 +-
arch/powerpc/kernel/module_64.c | 6 +++---
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/include/asm/elf.h b/arch/powerpc/include/asm/elf.h
index a4406714c060..bb0f278f9ed4 100644
--- a/arch/powerpc/include/asm/elf.h
+++ b/arch/powerpc/include/asm/elf.h
@@ -178,7 +178,7 @@ void relocate(unsigned long final_address);
/* There's actually a third entry here, but it's unused */
struct ppc64_opd_entry {
- unsigned long funcaddr;
+ unsigned long addr;
unsigned long r2;
};
diff --git a/arch/powerpc/include/asm/sections.h b/arch/powerpc/include/asm/sections.h
index 6e4af4492a14..32e7035863ac 100644
--- a/arch/powerpc/include/asm/sections.h
+++ b/arch/powerpc/include/asm/sections.h
@@ -77,7 +77,7 @@ static inline void *dereference_function_descriptor(void *ptr)
struct ppc64_opd_entry *desc = ptr;
void *p;
- if (!get_kernel_nofault(p, (void *)>funcaddr))
+ if (!get_kernel_nofault(p, (void *)>addr))
ptr = p;
return ptr;
}
diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
index 6baa676e7cb6..82908c9be627 100644
--- a/arch/powerpc/kernel/module_64.c
+++ b/arch/powerpc/kernel/module_64.c
@@ -72,11 +72,11 @@ static func_desc_t func_desc(unsigned long addr)
}
static unsigned long func_addr(unsigned long addr)
{
- return func_desc(addr).funcaddr;
+ return func_desc(addr).addr;
}
static unsigned long stub_func_addr(func_desc_t func)
{
- return func.funcaddr;
+ return func.addr;
}
static unsigned int local_entry_offset(const Elf64_Sym *sym)
{
@@ -187,7 +187,7 @@ static int relacmp(const void *_x, const void *_y)
static unsigned long get_stubs_size(const Elf64_Ehdr *hdr,
const Elf64_Shdr *sechdrs)
{
- /* One extra reloc so it's always 0-funcaddr terminated */
+ /* One extra reloc so it's always 0-addr terminated */
unsigned long relocs = 1;
unsigned i;
--
2.31.1
[PATCH][next] powerpc/vas: Fix potential NULL pointer dereference
(!ptr && !ptr->foo) strikes again. :)
The expression (!ptr && !ptr->foo) is bogus and in case ptr is NULL,
it leads to a NULL pointer dereference: ptr->foo.
Fix this by converting && to ||
This issue was detected with the help of Coccinelle, and audited and
fixed manually.
Fixes: 1a0d0d5ed5e3 ("powerpc/vas: Add platform specific user window
operations")
Cc: sta...@vger.kernel.org
Signed-off-by: Gustavo A. R. Silva
---
arch/powerpc/platforms/book3s/vas-api.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/platforms/book3s/vas-api.c
b/arch/powerpc/platforms/book3s/vas-api.c
index 30172e52e16b..4d82c92ddd52 100644
--- a/arch/powerpc/platforms/book3s/vas-api.c
+++ b/arch/powerpc/platforms/book3s/vas-api.c
@@ -303,7 +303,7 @@ static int coproc_ioc_tx_win_open(struct file *fp, unsigned
long arg)
return -EINVAL;
}
- if (!cp_inst->coproc->vops && !cp_inst->coproc->vops->open_win) {
+ if (!cp_inst->coproc->vops || !cp_inst->coproc->vops->open_win) {
pr_err("VAS API is not registered\n");
return -EACCES;
}
@@ -373,7 +373,7 @@ static int coproc_mmap(struct file *fp, struct
vm_area_struct *vma)
return -EINVAL;
}
- if (!cp_inst->coproc->vops && !cp_inst->coproc->vops->paste_addr) {
+ if (!cp_inst->coproc->vops || !cp_inst->coproc->vops->paste_addr) {
pr_err("%s(): VAS API is not registered\n", __func__);
return -EACCES;
}
--
2.27.0
Re: [PATCH v3 1/2] ftrace: disable preemption between ftrace_test_recursion_trylock/unlock()
On 2021/10/15 上午11:13, 王贇 wrote:
[snip]
>> # define do_ftrace_record_recursion(ip, pip)do { } while (0)
>> #endif
>>
>> +/*
>> + * trace_test_and_set_recursion() is called on several layers
>> + * of the ftrace code when handling the same ftrace entry.
>> + * These calls might be nested/recursive.
>> + *
>> + * It uses TRACE_LIST_*BITs to distinguish between this
>> + * internal recursion and recursion caused by calling
>> + * the traced function by the ftrace code.
>> + *
>> + * Returns: > 0 when no recursion
>> + * 0 when called recursively internally (safe)
>
> The 0 can also happened when ftrace handler recursively called trylock()
> under the same context, or not?
>
Never mind... you're right about this.
Regards,
Michael Wang
> Regards,
> Michael Wang
>
>> + * -1 when the traced function was called recursively from
>> + * the ftrace handler (unsafe)
>> + */
>> static __always_inline int trace_test_and_set_recursion(unsigned long ip,
>> unsigned long pip,
>> int start, int max)
>> {
>> unsigned int val = READ_ONCE(current->trace_recursion);
>> int bit;
>>
>> -/* A previous recursion check was made */
>> +/* Called recursively internally by different ftrace code layers? */
>> if ((val & TRACE_CONTEXT_MASK) > max)
>> return 0;
>
>>
>>
Re: [PATCH] ibmvscsi: use GFP_KERNEL with dma_alloc_coherent in initialize_event_pool
Tyrel Datwyler writes: > Just stumbled upon this trivial little patch that looks to have gotten lost in > the shuffle. Seems it even got a reviewed-by from Brian [1]. > > So, uh I guess after almost 3 years...ping? It's marked "Changes Requested" here: https://patchwork.kernel.org/project/linux-scsi/patch/1547089149-20577-1-git-send-email-tyr...@linux.vnet.ibm.com/ If it isn't picked up in a few days you should probably do a resend so that it reappears in patchwork. cheers
[powerpc:next-test] BUILD SUCCESS 3091f5fc5f1df7741ddf326561384e0997eca2a1
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next-test branch HEAD: 3091f5fc5f1df7741ddf326561384e0997eca2a1 powerpc: Mark .opd section read-only elapsed time: 811m configs tested: 127 configs skipped: 3 The following configs have been built successfully. More configs may be tested in the coming days. gcc tested configs: arm defconfig arm64allyesconfig arm64 defconfig arm allyesconfig arm allmodconfig i386 randconfig-c001-20211014 mips loongson1b_defconfig armmmp2_defconfig mips rb532_defconfig arm exynos_defconfig shedosk7705_defconfig armzeus_defconfig sh se7206_defconfig sh ap325rxa_defconfig s390 zfcpdump_defconfig mipsqi_lb60_defconfig arm colibri_pxa270_defconfig powerpc tqm8560_defconfig armqcom_defconfig arc axs103_smp_defconfig powerpc currituck_defconfig mips decstation_64_defconfig powerpc skiroot_defconfig xtensa virt_defconfig mips bigsur_defconfig arc alldefconfig mipsworkpad_defconfig mips loongson2k_defconfig arc haps_hs_smp_defconfig m68k atari_defconfig um defconfig powerpc tqm8xx_defconfig powerpcadder875_defconfig arcnsim_700_defconfig powerpc tqm8541_defconfig nios2 3c120_defconfig mips fuloong2e_defconfig powerpc lite5200b_defconfig powerpcwarp_defconfig openriscor1ksim_defconfig powerpc mpc8560_ads_defconfig armvexpress_defconfig powerpc pmac32_defconfig powerpc mpc8315_rdb_defconfig powerpcge_imp3a_defconfig powerpc sequoia_defconfig arm randconfig-c002-20211014 x86_64 randconfig-c001-20211014 ia64 allmodconfig ia64defconfig ia64 allyesconfig m68k allmodconfig m68kdefconfig m68k allyesconfig nios2 defconfig arc allyesconfig nds32 allnoconfig nds32 defconfig nios2allyesconfig cskydefconfig alpha defconfig alphaallyesconfig h8300allyesconfig arc defconfig sh allmodconfig xtensa allyesconfig parisc defconfig s390defconfig s390 allyesconfig s390 allmodconfig parisc allyesconfig sparcallyesconfig sparc defconfig i386defconfig i386 allyesconfig mips allyesconfig mips allmodconfig powerpc allyesconfig powerpc allmodconfig powerpc allnoconfig x86_64 randconfig-a006-20211014 x86_64 randconfig-a004-20211014 x86_64 randconfig-a001-20211014 x86_64 randconfig-a005-20211014 x86_64 randconfig-a002-20211014 x86_64 randconfig-a003-20211014 i386 randconfig-a003-20211014 i386 randconfig-a001-20211014 i386 randconfig-a005-20211014 i386 randconfig-a004-20211014 i386 randconfig-a002-20211014 i386 randconfig-a006-20211014 arc randconfig-r043-20211014 riscvnommu_k210_defconfig riscvallyesconfig riscvnommu_virt_defconfig riscv allnoconfig riscv defconfig riscv rv32_defconfig riscvallmodconfig x86_64rhel-8.3
Re: [PATCH v3 1/2] ftrace: disable preemption between ftrace_test_recursion_trylock/unlock()
On Thu, 14 Oct 2021 17:14:07 +0200 Petr Mladek wrote: > /** >* ftrace_test_recursion_trylock - tests for recursion in same context >* >* Use this for ftrace callbacks. This will detect if the function >* tracing recursed in the same context (normal vs interrupt), >* >* Returns: -1 if a recursion happened. > - * >= 0 if no recursion > + * >= 0 if no recursion (success) > + * > + * Disables the preemption on success. It is just for a convenience. > + * Current users needed to disable the preemtion for some reasons. >*/ I started replying to this explaining the difference between bit not zero and a bit zero, and I think I found a design flaw that can allow unwanted recursion. It's late and I'm about to go to bed, but I may have a new patch to fix this before this gets added, as the fix will conflict with this patch, and the fix will likely need to go to stable. Stay tuned. -- Steve
Re: [PATCH v3 1/2] ftrace: disable preemption between ftrace_test_recursion_trylock/unlock()
On 2021/10/14 下午11:14, Petr Mladek wrote:
[snip]
>> -return trace_test_and_set_recursion(ip, parent_ip, TRACE_FTRACE_START,
>> TRACE_FTRACE_MAX);
>> +int bit;
>> +
>> +bit = trace_test_and_set_recursion(ip, parent_ip, TRACE_FTRACE_START,
>> TRACE_FTRACE_MAX);
>> +/*
>> + * The zero bit indicate we are nested
>> + * in another trylock(), which means the
>> + * preemption already disabled.
>> + */
>> +if (bit > 0)
>> +preempt_disable_notrace();
>
> Is this safe? The preemption is disabled only when
> trace_test_and_set_recursion() was called by ftrace_test_recursion_trylock().
>
> We must either always disable the preemtion when bit >= 0.
> Or we have to disable the preemtion already in
> trace_test_and_set_recursion().
Internal calling of trace_test_and_set_recursion() will disable preemption
on succeed, it should be safe.
We can also consider move the logical into trace_test_and_set_recursion()
and trace_clear_recursion(), but I'm not very sure about that... ftrace
internally already make sure preemption disabled, what uncovered is those
users who call API trylock/unlock, isn't it?
>
>
> Finally, the comment confused me a lot. The difference between nesting and
> recursion is far from clear. And the code is tricky liky like hell :-)
> I propose to add some comments, see below for a proposal.
The comments do confusing, I'll make it something like:
The zero bit indicate trace recursion happened, whatever
the recursively call was made by ftrace handler or ftrace
itself, the preemption already disabled.
Will this one looks better to you?
>
>> +
>> +return bit;
>> }
>> /**
>> @@ -222,9 +233,13 @@ static __always_inline int
>> ftrace_test_recursion_trylock(unsigned long ip,
>> * @bit: The return of a successful ftrace_test_recursion_trylock()
>> *
>> * This is used at the end of a ftrace callback.
>> + *
>> + * Preemption will be enabled (if it was previously enabled).
>> */
>> static __always_inline void ftrace_test_recursion_unlock(int bit)
>> {
>> +if (bit)
>
> This is not symetric with trylock(). It should be:
>
> if (bit > 0)
>
> Anyway, trace_clear_recursion() quiently ignores bit != 0
Yes, bit == 0 should not happen in here.
>
>
>> +preempt_enable_notrace();
>> trace_clear_recursion(bit);
>> }
>
>
> Below is my proposed patch that tries to better explain the recursion
> check:
>
> From 20d69f11e2683262fa0043b803999061cbac543f Mon Sep 17 00:00:00 2001
> From: Petr Mladek
> Date: Thu, 14 Oct 2021 16:57:39 +0200
> Subject: [PATCH] trace: Better describe the recursion check return values
>
> The trace recursion check might be called recursively by different
> layers of the tracing code. It is safe recursion and the check
> is even optimized for this case.
>
> The problematic recursion is when the traced function is called
> by the tracing code. This is properly detected.
>
> Try to explain this difference a better way.
>
> Signed-off-by: Petr Mladek
> ---
> include/linux/trace_recursion.h | 16 +++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/trace_recursion.h b/include/linux/trace_recursion.h
> index a9f9c5714e65..b5efb804efdf 100644
> --- a/include/linux/trace_recursion.h
> +++ b/include/linux/trace_recursion.h
> @@ -159,13 +159,27 @@ extern void ftrace_record_recursion(unsigned long ip,
> unsigned long parent_ip);
> # define do_ftrace_record_recursion(ip, pip) do { } while (0)
> #endif
>
> +/*
> + * trace_test_and_set_recursion() is called on several layers
> + * of the ftrace code when handling the same ftrace entry.
> + * These calls might be nested/recursive.
> + *
> + * It uses TRACE_LIST_*BITs to distinguish between this
> + * internal recursion and recursion caused by calling
> + * the traced function by the ftrace code.
> + *
> + * Returns: > 0 when no recursion
> + * 0 when called recursively internally (safe)
The 0 can also happened when ftrace handler recursively called trylock()
under the same context, or not?
Regards,
Michael Wang
> + * -1 when the traced function was called recursively from
> + * the ftrace handler (unsafe)
> + */
> static __always_inline int trace_test_and_set_recursion(unsigned long ip,
> unsigned long pip,
> int start, int max)
> {
> unsigned int val = READ_ONCE(current->trace_recursion);
> int bit;
>
> - /* A previous recursion check was made */
> + /* Called recursively internally by different ftrace code layers? */
> if ((val & TRACE_CONTEXT_MASK) > max)
> return 0;
>
>
[powerpc:merge] BUILD SUCCESS 38947529bb05bbb8acfb2fe0ff96c2f1bc3f2c96
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git merge branch HEAD: 38947529bb05bbb8acfb2fe0ff96c2f1bc3f2c96 Automatic merge of 'next' into merge (2021-10-11 23:09) elapsed time: 5150m configs tested: 276 configs skipped: 3 The following configs have been built successfully. More configs may be tested in the coming days. gcc tested configs: arm defconfig arm64allyesconfig arm64 defconfig arm allyesconfig arm allmodconfig i386 randconfig-c001-20211012 i386 randconfig-c001-20211011 i386 randconfig-c001-20211014 mips tb0219_defconfig armmvebu_v7_defconfig xtensa audio_kc705_defconfig ia64zx1_defconfig powerpc tqm8560_defconfig sh se7705_defconfig m68k alldefconfig sh ul2_defconfig shhp6xx_defconfig powerpc mpc837x_rdb_defconfig mips sb1250_swarm_defconfig sh defconfig powerpc mpc83xx_defconfig sh sh7710voipgw_defconfig m68k hp300_defconfig arm at91_dt_defconfig mipsar7_defconfig arm cns3420vb_defconfig sh secureedge5410_defconfig powerpc tqm8540_defconfig powerpc motionpro_defconfig arm corgi_defconfig armvexpress_defconfig powerpc stx_gp3_defconfig sh sh7785lcr_32bit_defconfig powerpc obs600_defconfig powerpcgamecube_defconfig mips bmips_be_defconfig sh se7619_defconfig armmps2_defconfig h8300allyesconfig arm tegra_defconfig mips lemote2f_defconfig xtensa cadence_csp_defconfig powerpc ps3_defconfig powerpc xes_mpc85xx_defconfig shecovec24-romimage_defconfig sparc sparc64_defconfig shsh7757lcr_defconfig powerpc microwatt_defconfig mips ci20_defconfig openriscor1ksim_defconfig powerpc akebono_defconfig armqcom_defconfig sparcalldefconfig powerpcwarp_defconfig sh kfr2r09-romimage_defconfig arm u8500_defconfig powerpc tqm8548_defconfig um defconfig mipsmaltaup_defconfig pariscgeneric-32bit_defconfig microblaze defconfig shsh7785lcr_defconfig mips ip22_defconfig powerpc mpc866_ads_defconfig powerpcge_imp3a_defconfig xtensageneric_kc705_defconfig ia64 alldefconfig nios2 3c120_defconfig mips fuloong2e_defconfig sh sdk7786_defconfig powerpc canyonlands_defconfig nds32 defconfig mips cavium_octeon_defconfig sparc sparc32_defconfig powerpc mpc8272_ads_defconfig powerpc cm5200_defconfig m68km5272c3_defconfig riscvnommu_k210_defconfig xtensaxip_kc705_defconfig powerpc linkstation_defconfig arc haps_hs_defconfig m68k m5249evb_defconfig mipsvocore2_defconfig m68k m5208evb_defconfig ia64 bigsur_defconfig arc nsimosci_hs_defconfig arm tct_hammer_defconfig mips rbtx49xx_defconfig powerpc pmac32_defconfig sh lboxre2_defconfig powerpc ep88xc_defconfig sh kfr2r09_defconfig powerpc mgcoge_defconfig arm simpad_defconfig powerpc ebony_defconfig xtensa alldefconfig powerpc makalu_defconfig armmulti_v7_defconfig sh se7721_defconfig arm gemini_defconfig powerpc
[PATCH v11 2/3] tty: hvc: pass DMA capable memory to put_chars()
As well known, hvc backend can register its opertions to hvc backend.
the operations contain put_chars(), get_chars() and so on.
Some hvc backend may do dma in its operations. eg, put_chars() of
virtio-console. But in the code of hvc framework, it may pass DMA
incapable memory to put_chars() under a specific configuration, which
is explained in commit c4baad5029(virtio-console: avoid DMA from stack):
1, c[] is on stack,
hvc_console_print():
char c[N_OUTBUF] __ALIGNED__;
cons_ops[index]->put_chars(vtermnos[index], c, i);
2, ch is on stack,
static void hvc_poll_put_char(,,char ch)
{
struct tty_struct *tty = driver->ttys[0];
struct hvc_struct *hp = tty->driver_data;
int n;
do {
n = hp->ops->put_chars(hp->vtermno, , 1);
} while (n <= 0);
}
Commit c4baad5029 is just the fix to avoid DMA from stack memory, which
is passed to virtio-console by hvc framework in above code. But I think
the fix is aggressive, it directly uses kmemdup() to alloc new buffer
from kmalloc area and do memcpy no matter the memory is in kmalloc area
or not. But most importantly, it should better be fixed in the hvc
framework, by changing it to never pass stack memory to the put_chars()
function in the first place. Otherwise, we still face the same issue if
a new hvc backend using dma added in the furture.
In this patch, add 'char cons_outbuf[]' as part of 'struct hvc_struct',
so hp->cons_outbuf is no longer the stack memory, we can use it in above
cases safely. We also add lock to protect cons_outbuf instead of using
the global lock of hvc.
Introduce another array(cons_hvcs[]) for hvc pointers next to the
cons_ops[] and vtermnos[] arrays. With the array, we can easily find
hvc's cons_outbuf and its lock.
With the patch, we can revert the fix c4baad5029.
Signed-off-by: Xianting Tian
Signed-off-by: Shile Zhang
---
drivers/tty/hvc/hvc_console.c | 36 ---
drivers/tty/hvc/hvc_console.h | 21 +++-
2 files changed, 41 insertions(+), 16 deletions(-)
diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c
index 5957ab728..11f2463a1 100644
--- a/drivers/tty/hvc/hvc_console.c
+++ b/drivers/tty/hvc/hvc_console.c
@@ -41,16 +41,6 @@
*/
#define HVC_CLOSE_WAIT (HZ/100) /* 1/10 of a second */
-/*
- * These sizes are most efficient for vio, because they are the
- * native transfer size. We could make them selectable in the
- * future to better deal with backends that want other buffer sizes.
- */
-#define N_OUTBUF 16
-#define N_INBUF16
-
-#define __ALIGNED__ __attribute__((__aligned__(L1_CACHE_BYTES)))
-
static struct tty_driver *hvc_driver;
static struct task_struct *hvc_task;
@@ -142,6 +132,7 @@ static int hvc_flush(struct hvc_struct *hp)
static const struct hv_ops *cons_ops[MAX_NR_HVC_CONSOLES];
static uint32_t vtermnos[MAX_NR_HVC_CONSOLES] =
{[0 ... MAX_NR_HVC_CONSOLES - 1] = -1};
+static struct hvc_struct *cons_hvcs[MAX_NR_HVC_CONSOLES];
/*
* Console APIs, NOT TTY. These APIs are available immediately when
@@ -151,9 +142,11 @@ static uint32_t vtermnos[MAX_NR_HVC_CONSOLES] =
static void hvc_console_print(struct console *co, const char *b,
unsigned count)
{
- char c[N_OUTBUF] __ALIGNED__;
+ char *c;
unsigned i = 0, n = 0;
int r, donecr = 0, index = co->index;
+ unsigned long flags;
+ struct hvc_struct *hp;
/* Console access attempt outside of acceptable console range. */
if (index >= MAX_NR_HVC_CONSOLES)
@@ -163,6 +156,13 @@ static void hvc_console_print(struct console *co, const
char *b,
if (vtermnos[index] == -1)
return;
+ hp = cons_hvcs[index];
+ if (!hp)
+ return;
+
+ c = hp->cons_outbuf;
+
+ spin_lock_irqsave(>cons_outbuf_lock, flags);
while (count > 0 || i > 0) {
if (count > 0 && i < sizeof(c)) {
if (b[n] == '\n' && !donecr) {
@@ -191,6 +191,7 @@ static void hvc_console_print(struct console *co, const
char *b,
}
}
}
+ spin_unlock_irqrestore(>cons_outbuf_lock, flags);
hvc_console_flush(cons_ops[index], vtermnos[index]);
}
@@ -878,9 +879,13 @@ static void hvc_poll_put_char(struct tty_driver *driver,
int line, char ch)
struct tty_struct *tty = driver->ttys[0];
struct hvc_struct *hp = tty->driver_data;
int n;
+ unsigned long flags;
do {
- n = hp->ops->put_chars(hp->vtermno, , 1);
+ spin_lock_irqsave(>cons_outbuf_lock, flags);
+ hp->cons_outbuf[0] = ch;
+ n = hp->ops->put_chars(hp->vtermno, >cons_outbuf[0], 1);
+ spin_unlock_irqrestore(>cons_outbuf_lock, flags);
} while (n <= 0);
}
#endif
@@ -922,8 +927,7 @@ struct hvc_struct *hvc_alloc(uint32_t vtermno, int data,
[PATCH v11 3/3] virtio-console: remove unnecessary kmemdup()
This revert commit c4baad5029 ("virtio-console: avoid DMA from stack")
hvc framework will never pass stack memory to the put_chars() function,
So the calling of kmemdup() is unnecessary, we can remove it.
Signed-off-by: Xianting Tian
Reviewed-by: Shile Zhang
---
drivers/char/virtio_console.c | 12 ++--
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 7eaf303a7..4ed3ffb1d 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -1117,8 +1117,6 @@ static int put_chars(u32 vtermno, const char *buf, int
count)
{
struct port *port;
struct scatterlist sg[1];
- void *data;
- int ret;
if (unlikely(early_put_chars))
return early_put_chars(vtermno, buf, count);
@@ -1127,14 +1125,8 @@ static int put_chars(u32 vtermno, const char *buf, int
count)
if (!port)
return -EPIPE;
- data = kmemdup(buf, count, GFP_ATOMIC);
- if (!data)
- return -ENOMEM;
-
- sg_init_one(sg, data, count);
- ret = __send_to_port(port, sg, 1, count, data, false);
- kfree(data);
- return ret;
+ sg_init_one(sg, buf, count);
+ return __send_to_port(port, sg, 1, count, (void *)buf, false);
}
/*
--
2.17.1
[PATCH v11 0/3] make hvc pass dma capable memory to its backend
Dear all,
This patch series make hvc framework pass DMA capable memory to
put_chars() of hvc backend(eg, virtio-console), and revert commit
c4baad5029 ("virtio-console: avoid DMA from stack”)
V1
virtio-console: avoid DMA from vmalloc area
https://lkml.org/lkml/2021/7/27/494
For v1 patch, Arnd Bergmann suggests to fix the issue in the first
place:
Make hvc pass DMA capable memory to put_chars()
The fix suggestion is included in v2.
V2
[PATCH 1/2] tty: hvc: pass DMA capable memory to put_chars()
https://lkml.org/lkml/2021/8/1/8
[PATCH 2/2] virtio-console: remove unnecessary kmemdup()
https://lkml.org/lkml/2021/8/1/9
For v2 patch, Arnd Bergmann suggests to make new buf part of the
hvc_struct structure, and fix the compile issue.
The fix suggestion is included in v3.
V3
[PATCH v3 1/2] tty: hvc: pass DMA capable memory to put_chars()
https://lkml.org/lkml/2021/8/3/1347
[PATCH v3 2/2] virtio-console: remove unnecessary kmemdup()
https://lkml.org/lkml/2021/8/3/1348
For v3 patch, Jiri Slaby suggests to make 'char c[N_OUTBUF]' part of
hvc_struct, and make 'hp->outbuf' aligned and use struct_size() to
calculate the size of hvc_struct. The fix suggestion is included in
v4.
V4
[PATCH v4 0/2] make hvc pass dma capable memory to its backend
https://lkml.org/lkml/2021/8/5/1350
[PATCH v4 1/2] tty: hvc: pass DMA capable memory to put_chars()
https://lkml.org/lkml/2021/8/5/1351
[PATCH v4 2/2] virtio-console: remove unnecessary kmemdup()
https://lkml.org/lkml/2021/8/5/1352
For v4 patch, Arnd Bergmann suggests to introduce another
array(cons_outbuf[]) for the buffer pointers next to the cons_ops[]
and vtermnos[] arrays. This fix included in this v5 patch.
V5
Arnd Bergmann suggests to use "L1_CACHE_BYTES" as dma alignment,
use 'sizeof(long)' as dma alignment is wrong. fix it in v6.
V6
It contains coding error, fix it in v7 and it worked normally
according to test result.
V7
Greg KH suggests to add test and code review developer,
Jiri Slaby suggests to use lockless buffer and fix dma alignment
in separate patch.
fix above things in v8.
V8
This contains coding error when switch to use new buffer. fix it in v9.
V9
It didn't make things much clearer, it needs add more comments for new added
buf.
Add use lock to protect new added buffer. fix in v10.
V10
Remove 'char outchar' and its lock from hvc_struct, adjust hvc_struct and use
pahole to display the struct layout.
fix it in v11.
TEST STEPS*
1, config guest console=hvc0
2, start guest
3, login guest
Welcome to Buildroot
buildroot login: root
#
# cat /proc/cmdline
console=hvc0 root=/dev/vda rw init=/sbin/init
#
drivers/tty/hvc/hvc_console.c | 36 ---
drivers/tty/hvc/hvc_console.h | 21 +++-
drivers/char/virtio_console.c | 12 ++--
3 file changed
[PATCH v11 1/3] tty: hvc: use correct dma alignment size
Use L1_CACHE_BYTES as the dma alignment size, use 'sizeof(long)' as dma alignment is wrong. Signed-off-by: Xianting Tian Signed-off-by: Shile Zhang --- drivers/tty/hvc/hvc_console.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c index 5bb8c4e44..5957ab728 100644 --- a/drivers/tty/hvc/hvc_console.c +++ b/drivers/tty/hvc/hvc_console.c @@ -49,7 +49,7 @@ #define N_OUTBUF 16 #define N_INBUF16 -#define __ALIGNED__ __attribute__((__aligned__(sizeof(long +#define __ALIGNED__ __attribute__((__aligned__(L1_CACHE_BYTES))) static struct tty_driver *hvc_driver; static struct task_struct *hvc_task; -- 2.17.1
[powerpc:fixes-test] BUILD SUCCESS 6f779e1d359b8d5801f677c1d49dcfa10bf95674
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git fixes-test branch HEAD: 6f779e1d359b8d5801f677c1d49dcfa10bf95674 powerpc/xive: Discard disabled interrupts in get_irqchip_state() elapsed time: 728m configs tested: 99 configs skipped: 94 The following configs have been built successfully. More configs may be tested in the coming days. gcc tested configs: arm defconfig arm64allyesconfig arm64 defconfig i386 randconfig-c001-20211014 armzeus_defconfig sh se7206_defconfig sh ap325rxa_defconfig armqcom_defconfig arc axs103_smp_defconfig powerpc currituck_defconfig mips decstation_64_defconfig powerpc skiroot_defconfig mipsworkpad_defconfig mips loongson2k_defconfig arc haps_hs_smp_defconfig m68k atari_defconfig um defconfig powerpc tqm8xx_defconfig powerpcadder875_defconfig arcnsim_700_defconfig powerpc tqm8541_defconfig nios2 3c120_defconfig mips fuloong2e_defconfig powerpcwarp_defconfig openriscor1ksim_defconfig powerpcge_imp3a_defconfig powerpc lite5200b_defconfig powerpc sequoia_defconfig arm randconfig-c002-20211014 x86_64 randconfig-c001-20211014 ia64 allmodconfig ia64defconfig ia64 allyesconfig m68k allmodconfig m68kdefconfig m68k allyesconfig nios2 defconfig nds32 allnoconfig nds32 defconfig cskydefconfig alpha defconfig alphaallyesconfig nios2allyesconfig h8300allyesconfig arc defconfig sh allmodconfig parisc defconfig s390defconfig sparcallyesconfig sparc defconfig i386defconfig mips allyesconfig mips allmodconfig powerpc allyesconfig powerpc allmodconfig powerpc allnoconfig i386 randconfig-a003-20211014 i386 randconfig-a001-20211014 i386 randconfig-a005-20211014 i386 randconfig-a004-20211014 i386 randconfig-a002-20211014 i386 randconfig-a006-20211014 x86_64 randconfig-a006-20211014 x86_64 randconfig-a004-20211014 x86_64 randconfig-a001-20211014 x86_64 randconfig-a005-20211014 x86_64 randconfig-a002-20211014 x86_64 randconfig-a003-20211014 riscvnommu_k210_defconfig riscvnommu_virt_defconfig riscv allnoconfig riscv defconfig riscv rv32_defconfig x86_64rhel-8.3-kselftests um x86_64_defconfig um i386_defconfig x86_64 defconfig x86_64 rhel-8.3 x86_64 kexec clang tested configs: arm randconfig-c002-20211014 i386 randconfig-c001-20211014 s390 randconfig-c005-20211014 x86_64 randconfig-c007-20211014 powerpc randconfig-c003-20211014 riscvrandconfig-c006-20211014 x86_64 randconfig-a012-20211014 x86_64 randconfig-a015-20211014 x86_64 randconfig-a016-20211014 x86_64 randconfig-a014-20211014 x86_64 randconfig-a011-20211014 x86_64 randconfig-a013-20211014 i386 randconfig-a016-20211014 i386 randconfig-a014-20211014 i386 randconfig-a011-20211014 i386 randconfig-a015-20211014 i386 randconfig-a012-20211014 i386 randconfig-a013-20211014 hexagon randconfig-r041-20211014 s390 randconfig-r044-20211014 riscv
Re: [PATCH v2] KVM: PPC: Defer vtime accounting 'til after IRQ handling
Excerpts from Laurent Vivier's message of October 13, 2021 7:30 pm:
> On 13/10/2021 01:18, Michael Ellerman wrote:
>> Laurent Vivier writes:
>>> Commit 112665286d08 moved guest_exit() in the interrupt protected
>>> area to avoid wrong context warning (or worse), but the tick counter
>>> cannot be updated and the guest time is accounted to the system time.
>>>
>>> To fix the problem port to POWER the x86 fix
>>> 160457140187 ("Defer vtime accounting 'til after IRQ handling"):
>>>
>>> "Defer the call to account guest time until after servicing any IRQ(s)
>>> that happened in the guest or immediately after VM-Exit. Tick-based
>>> accounting of vCPU time relies on PF_VCPU being set when the tick IRQ
>>> handler runs, and IRQs are blocked throughout the main sequence of
>>> vcpu_enter_guest(), including the call into vendor code to actually
>>> enter and exit the guest."
>>>
>>> Fixes: 112665286d08 ("KVM: PPC: Book3S HV: Context tracking exit guest
>>> context before enabling irqs")
>>> Cc: npig...@gmail.com
>>> Cc: # 5.12
>>> Signed-off-by: Laurent Vivier
>>> ---
>>>
>>> Notes:
>>> v2: remove reference to commit 61bd0f66ff92
>>> cc stable 5.12
>>> add the same comment in the code as for x86
>>>
>>> arch/powerpc/kvm/book3s_hv.c | 24
>>> 1 file changed, 20 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
>>> index 2acb1c96cfaf..a694d1a8f6ce 100644
>>> --- a/arch/powerpc/kvm/book3s_hv.c
>>> +++ b/arch/powerpc/kvm/book3s_hv.c
>> ...
>>> @@ -4506,13 +4514,21 @@ int kvmhv_run_single_vcpu(struct kvm_vcpu *vcpu,
>>> u64 time_limit,
>>>
>>> srcu_read_unlock(>srcu, srcu_idx);
>>>
>>> + context_tracking_guest_exit();
>>> +
>>> set_irq_happened(trap);
>>>
>>> kvmppc_set_host_core(pcpu);
>>>
>>> - guest_exit_irqoff();
>>> -
>>> local_irq_enable();
>>> + /*
>>> +* Wait until after servicing IRQs to account guest time so that any
>>> +* ticks that occurred while running the guest are properly accounted
>>> +* to the guest. Waiting until IRQs are enabled degrades the accuracy
>>> +* of accounting via context tracking, but the loss of accuracy is
>>> +* acceptable for all known use cases.
>>> +*/
>>> + vtime_account_guest_exit();
>>
>> This pops a warning for me, running guest(s) on Power8:
>>
>>[ 270.745303][T16661] [ cut here ]
>>[ 270.745374][T16661] WARNING: CPU: 72 PID: 16661 at
>> arch/powerpc/kernel/time.c:311 vtime_account_kernel+0xe0/0xf0
>
> Thank you, I missed that...
>
> My patch is wrong, I have to add vtime_account_guest_exit() before the
> local_irq_enable().
I thought so because if we take an interrupt after exiting the guest that
should be accounted to kernel not guest.
>
> arch/powerpc/kernel/time.c
>
> 305 static unsigned long vtime_delta(struct cpu_accounting_data *acct,
> 306 unsigned long *stime_scaled,
> 307 unsigned long *steal_time)
> 308 {
> 309 unsigned long now, stime;
> 310
> 311 WARN_ON_ONCE(!irqs_disabled());
> ...
>
> But I don't understand how ticks can be accounted now if irqs are still
> disabled.
>
> Not sure it is as simple as expected...
I don't know all the timer stuff too well. The
!CONFIG_VIRT_CPU_ACCOUNTING case is relying on PF_VCPU to be set when
the host timer interrupt runs irqtime_account_process_tick runs so it
can accumulate that tick to the guest?
That probably makes sense then, but it seems like we need that in a
different place. Timer interrupts are not guaranteed to be the first one
to occur when interrupts are enabled.
Maybe a new tick_account_guest_exit() and move PF_VCPU clearing to that
for tick based accounting. Call it after local_irq_enable and call the
vtime accounting before it. Would that work?
Thanks,
Nick
[PATCH v2] powerpc/64s: Default to 64K pages for 64 bit book3s
For 64-bit book3s the default should be 64K as that's what modern CPUs are designed for. The following defconfigs already set CONFIG_PPC_64K_PAGES: cell_defconfig pasemi_defconfig powernv_defconfig ppc64_defconfig pseries_defconfig skiroot_defconfig The have the option removed from the defconfig, as it is now the default. The defconfigs that now need to set CONFIG_PPC_4K_PAGES to maintain their existing behaviour are: g5_defconfig maple_defconfig microwatt_defconfig ps3_defconfig Link: https://github.com/linuxppc/issues/issues/109 Signed-off-by: Joel Stanley --- v2: remove unrelated change from microwatt_defconfig arch/powerpc/Kconfig | 1 + arch/powerpc/configs/cell_defconfig | 1 - arch/powerpc/configs/g5_defconfig| 1 + arch/powerpc/configs/maple_defconfig | 1 + arch/powerpc/configs/microwatt_defconfig | 1 + arch/powerpc/configs/pasemi_defconfig| 1 - arch/powerpc/configs/powernv_defconfig | 1 - arch/powerpc/configs/ppc64_defconfig | 1 - arch/powerpc/configs/ps3_defconfig | 1 + arch/powerpc/configs/pseries_defconfig | 1 - arch/powerpc/configs/skiroot_defconfig | 1 - 11 files changed, 5 insertions(+), 6 deletions(-) diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig index 8a584414ef67..e2c220fa91c0 100644 --- a/arch/powerpc/Kconfig +++ b/arch/powerpc/Kconfig @@ -708,6 +708,7 @@ config ARCH_MEMORY_PROBE choice prompt "Page size" + default PPC_64K_PAGES if PPC_BOOK3S_64 default PPC_4K_PAGES help Select the kernel logical page size. Increasing the page size diff --git a/arch/powerpc/configs/cell_defconfig b/arch/powerpc/configs/cell_defconfig index cc2c0d51f493..7fd9e596ea33 100644 --- a/arch/powerpc/configs/cell_defconfig +++ b/arch/powerpc/configs/cell_defconfig @@ -36,7 +36,6 @@ CONFIG_GEN_RTC=y CONFIG_BINFMT_MISC=m CONFIG_IRQ_ALL_CPUS=y CONFIG_NUMA=y -CONFIG_PPC_64K_PAGES=y CONFIG_SCHED_SMT=y CONFIG_PCIEPORTBUS=y CONFIG_NET=y diff --git a/arch/powerpc/configs/g5_defconfig b/arch/powerpc/configs/g5_defconfig index 63d611cc160f..9d6212a8b195 100644 --- a/arch/powerpc/configs/g5_defconfig +++ b/arch/powerpc/configs/g5_defconfig @@ -26,6 +26,7 @@ CONFIG_CPU_FREQ_PMAC64=y CONFIG_GEN_RTC=y CONFIG_KEXEC=y CONFIG_IRQ_ALL_CPUS=y +CONFIG_PPC_4K_PAGES=y CONFIG_PCI_MSI=y CONFIG_NET=y CONFIG_PACKET=y diff --git a/arch/powerpc/configs/maple_defconfig b/arch/powerpc/configs/maple_defconfig index 9424c1e67e1c..c821a97f4a89 100644 --- a/arch/powerpc/configs/maple_defconfig +++ b/arch/powerpc/configs/maple_defconfig @@ -25,6 +25,7 @@ CONFIG_UDBG_RTAS_CONSOLE=y CONFIG_GEN_RTC=y CONFIG_KEXEC=y CONFIG_IRQ_ALL_CPUS=y +CONFIG_PPC_4K_PAGES=y CONFIG_PCI_MSI=y CONFIG_NET=y CONFIG_PACKET=y diff --git a/arch/powerpc/configs/microwatt_defconfig b/arch/powerpc/configs/microwatt_defconfig index 9465209b8c5b..07d87a4044b2 100644 --- a/arch/powerpc/configs/microwatt_defconfig +++ b/arch/powerpc/configs/microwatt_defconfig @@ -26,6 +26,7 @@ CONFIG_PPC_MICROWATT=y # CONFIG_PPC_OF_BOOT_TRAMPOLINE is not set CONFIG_CPU_FREQ=y CONFIG_HZ_100=y +CONFIG_PPC_4K_PAGES=y # CONFIG_PPC_MEM_KEYS is not set # CONFIG_SECCOMP is not set # CONFIG_MQ_IOSCHED_KYBER is not set diff --git a/arch/powerpc/configs/pasemi_defconfig b/arch/powerpc/configs/pasemi_defconfig index 78606b7e42df..e00a703581c3 100644 --- a/arch/powerpc/configs/pasemi_defconfig +++ b/arch/powerpc/configs/pasemi_defconfig @@ -22,7 +22,6 @@ CONFIG_CPU_FREQ_GOV_POWERSAVE=y CONFIG_CPU_FREQ_GOV_USERSPACE=y CONFIG_CPU_FREQ_GOV_ONDEMAND=y CONFIG_HZ_1000=y -CONFIG_PPC_64K_PAGES=y # CONFIG_SECCOMP is not set CONFIG_PCI_MSI=y CONFIG_PCCARD=y diff --git a/arch/powerpc/configs/powernv_defconfig b/arch/powerpc/configs/powernv_defconfig index 8bfeea6c7de7..49f49c263935 100644 --- a/arch/powerpc/configs/powernv_defconfig +++ b/arch/powerpc/configs/powernv_defconfig @@ -62,7 +62,6 @@ CONFIG_MEMORY_FAILURE=y CONFIG_HWPOISON_INJECT=m CONFIG_TRANSPARENT_HUGEPAGE=y CONFIG_DEFERRED_STRUCT_PAGE_INIT=y -CONFIG_PPC_64K_PAGES=y CONFIG_SCHED_SMT=y CONFIG_PM=y CONFIG_HOTPLUG_PCI=y diff --git a/arch/powerpc/configs/ppc64_defconfig b/arch/powerpc/configs/ppc64_defconfig index 0ad2291337a7..203d0b7f0bb8 100644 --- a/arch/powerpc/configs/ppc64_defconfig +++ b/arch/powerpc/configs/ppc64_defconfig @@ -52,7 +52,6 @@ CONFIG_KEXEC_FILE=y CONFIG_CRASH_DUMP=y CONFIG_FA_DUMP=y CONFIG_IRQ_ALL_CPUS=y -CONFIG_PPC_64K_PAGES=y CONFIG_SCHED_SMT=y CONFIG_HOTPLUG_PCI=y CONFIG_HOTPLUG_PCI_RPA=m diff --git a/arch/powerpc/configs/ps3_defconfig b/arch/powerpc/configs/ps3_defconfig index f300dcb937cc..7c95fab4b920 100644 --- a/arch/powerpc/configs/ps3_defconfig +++ b/arch/powerpc/configs/ps3_defconfig @@ -30,6 +30,7 @@ CONFIG_PS3_LPM=m # CONFIG_CORE_DUMP_DEFAULT_ELF_HEADERS is not set CONFIG_BINFMT_MISC=y CONFIG_KEXEC=y +CONFIG_PPC_4K_PAGES=y # CONFIG_SPARSEMEM_VMEMMAP is not set # CONFIG_COMPACTION is not set CONFIG_SCHED_SMT=y diff --git
Re: [PATCH] ibmvscsi: use GFP_KERNEL with dma_alloc_coherent in initialize_event_pool
Just stumbled upon this trivial little patch that looks to have gotten lost in
the shuffle. Seems it even got a reviewed-by from Brian [1].
So, uh I guess after almost 3 years...ping?
-Tyrel
[1]
https://yhbt.net/lore/all/fd33df0e-012b-e437-c6e9-29cd08838...@linux.vnet.ibm.com/
On 01/09/2019 08:59 PM, Tyrel Datwyler wrote:
> During driver probe we allocate a dma region for our event pool.
> Currently, zero is passed for the gfp_flags parameter. Driver probe
> callbacks run in process context and we hold no locks so we can sleep
> here if necessary.
>
> Fix by passing GFP_KERNEL explicitly to dma_alloc_coherent().
>
> Signed-off-by: Tyrel Datwyler
> ---
> drivers/scsi/ibmvscsi/ibmvscsi.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c
> b/drivers/scsi/ibmvscsi/ibmvscsi.c
> index cb8535e..10d5e77 100644
> --- a/drivers/scsi/ibmvscsi/ibmvscsi.c
> +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
> @@ -465,7 +465,7 @@ static int initialize_event_pool(struct event_pool *pool,
> pool->iu_storage =
> dma_alloc_coherent(hostdata->dev,
> pool->size * sizeof(*pool->iu_storage),
> ->iu_token, 0);
> +>iu_token, GFP_KERNEL);
> if (!pool->iu_storage) {
> kfree(pool->events);
> return -ENOMEM;
>
Re: [PATCH] powerpc/mpc512x: dts: fix PSC node warnings
On Thu, 14 Oct 2021 07:33:26 -0500 Rob Herring robh...@kernel.org wrote: ... >> +++ b/arch/powerpc/boot/dts/ac14xx.dts >> @@ -301,13 +301,21 @@ >> fsl,tx-fifo-size = <512>; >> }; >> >> + /delete-node/ psc@11400; >> + /delete-node/ psc@11500; > >That's an odd way to fix this, and means every user of the .dtsi file >with these nodes will have to repeat the same thing. okay, in v2 patch I've extracted the psc nodes to files which can be included and extended individually. Anatolij
[PATCH v2] powerpc/mpc512x: dts: fix PSC node warnings
Rework PSC node description to fix build warnings like:
mpc5121.dtsi:397.13-406.5: Warning (spi_bus_bridge): /soc@8000/psc@11400:
node name for SPI buses should be 'spi'
mpc5121.dtsi:409.13-418.5: Warning (spi_bus_bridge): /soc@8000/psc@11500:
node name for SPI buses should be 'spi'
mpc5121.dtsi:457.13-466.5: Warning (spi_bus_bridge): /soc@8000/psc@11900:
node name for SPI buses should be 'spi'
Signed-off-by: Anatolij Gustschin
---
Changes in v2:
- extract PSC nodes to files which can be included
separately and extended as needed
arch/powerpc/boot/dts/ac14xx.dts| 118
arch/powerpc/boot/dts/mpc5121-psc0.dtsi | 16 +++
arch/powerpc/boot/dts/mpc5121-psc1.dtsi | 15 ++
arch/powerpc/boot/dts/mpc5121-psc10.dtsi| 15 ++
arch/powerpc/boot/dts/mpc5121-psc11.dtsi| 15 ++
arch/powerpc/boot/dts/mpc5121-psc2.dtsi | 15 ++
arch/powerpc/boot/dts/mpc5121-psc3.dtsi | 15 ++
arch/powerpc/boot/dts/mpc5121-psc4-spi.dtsi | 17 +++
arch/powerpc/boot/dts/mpc5121-psc4.dtsi | 15 ++
arch/powerpc/boot/dts/mpc5121-psc5-spi.dtsi | 17 +++
arch/powerpc/boot/dts/mpc5121-psc5.dtsi | 15 ++
arch/powerpc/boot/dts/mpc5121-psc6.dtsi | 15 ++
arch/powerpc/boot/dts/mpc5121-psc7.dtsi | 15 ++
arch/powerpc/boot/dts/mpc5121-psc8.dtsi | 15 ++
arch/powerpc/boot/dts/mpc5121-psc9-spi.dtsi | 17 +++
arch/powerpc/boot/dts/mpc5121-psc9.dtsi | 15 ++
arch/powerpc/boot/dts/mpc5121.dtsi | 148 +---
arch/powerpc/boot/dts/mpc5121ads.dts| 42 +++---
arch/powerpc/boot/dts/pdm360ng.dts | 104 +++---
19 files changed, 371 insertions(+), 273 deletions(-)
create mode 100644 arch/powerpc/boot/dts/mpc5121-psc0.dtsi
create mode 100644 arch/powerpc/boot/dts/mpc5121-psc1.dtsi
create mode 100644 arch/powerpc/boot/dts/mpc5121-psc10.dtsi
create mode 100644 arch/powerpc/boot/dts/mpc5121-psc11.dtsi
create mode 100644 arch/powerpc/boot/dts/mpc5121-psc2.dtsi
create mode 100644 arch/powerpc/boot/dts/mpc5121-psc3.dtsi
create mode 100644 arch/powerpc/boot/dts/mpc5121-psc4-spi.dtsi
create mode 100644 arch/powerpc/boot/dts/mpc5121-psc4.dtsi
create mode 100644 arch/powerpc/boot/dts/mpc5121-psc5-spi.dtsi
create mode 100644 arch/powerpc/boot/dts/mpc5121-psc5.dtsi
create mode 100644 arch/powerpc/boot/dts/mpc5121-psc6.dtsi
create mode 100644 arch/powerpc/boot/dts/mpc5121-psc7.dtsi
create mode 100644 arch/powerpc/boot/dts/mpc5121-psc8.dtsi
create mode 100644 arch/powerpc/boot/dts/mpc5121-psc9-spi.dtsi
create mode 100644 arch/powerpc/boot/dts/mpc5121-psc9.dtsi
diff --git a/arch/powerpc/boot/dts/ac14xx.dts b/arch/powerpc/boot/dts/ac14xx.dts
index 5d8877e1f4ad..0af3b0ab7550 100644
--- a/arch/powerpc/boot/dts/ac14xx.dts
+++ b/arch/powerpc/boot/dts/ac14xx.dts
@@ -15,8 +15,8 @@
#size-cells = <1>;
aliases {
- serial0 =
- serial1 =
+ serial0 =
+ serial1 =
spi4 =
spi5 =
};
@@ -294,62 +294,6 @@
status = "disabled";
};
- /* PSC3 serial port A, aka ttyPSC0 */
- serial0: psc@11300 {
- compatible = "fsl,mpc5121-psc-uart", "fsl,mpc5121-psc";
- fsl,rx-fifo-size = <512>;
- fsl,tx-fifo-size = <512>;
- };
-
- /* PSC4 in SPI mode */
- spi4: psc@11400 {
- compatible = "fsl,mpc5121-psc-spi", "fsl,mpc5121-psc";
- fsl,rx-fifo-size = <768>;
- fsl,tx-fifo-size = <768>;
- #address-cells = <1>;
- #size-cells = <0>;
- num-cs = <1>;
- cs-gpios = <_pic 25 0>;
-
- flash: m25p128@0 {
- compatible = "st,m25p128";
- spi-max-frequency = <2000>;
- reg = <0>;
- #address-cells = <1>;
- #size-cells = <1>;
-
- partition@0 {
- label = "spi-flash0";
- reg = <0x 0x0100>;
- };
- };
- };
-
- /* PSC5 in SPI mode */
- spi5: psc@11500 {
- compatible = "fsl,mpc5121-psc-spi", "fsl,mpc5121-psc";
- fsl,mode = "spi-master";
- fsl,rx-fifo-size = <128>;
- fsl,tx-fifo-size = <128>;
- #address-cells = <1>;
- #size-cells = <0>;
-
- lcd@0 {
- compatible = "ilitek,ili922x";
- reg = <0>;
-
Re: [PATCH v2 03/13] powerpc: Remove func_descr_t
Christophe Leroy writes:
> 'func_descr_t' is redundant with 'struct ppc64_opd_entry'
So, if I understand the overall direction of the series, you're
consolidating powerpc around one single type for function descriptors,
and then you're creating a generic typedef so that generic code can
always do ((func_desc_t)x)->addr to get the address of a function out of
a function descriptor regardless of arch. (And regardless of whether the
arch uses function descriptors or not.)
So:
- why pick ppc64_opd_entry over func_descr_t?
- Why not make our struct just called func_desc_t - why have a
ppc64_opd_entry type or a func_descr_t typedef?
- Should this patch wait until after you've made the generic
func_desc_t change and move directly to that new interface? (rather
than move from func_descr_t -> ppc64_opd_entry -> ...) Or is there a
particular reason arch specific code should use an arch-specific
struct or named type?
> Remove it.
>
> Signed-off-by: Christophe Leroy
> ---
> arch/powerpc/include/asm/code-patching.h | 2 +-
> arch/powerpc/include/asm/types.h | 6 --
> arch/powerpc/kernel/signal_64.c | 8
> 3 files changed, 5 insertions(+), 11 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/code-patching.h
> b/arch/powerpc/include/asm/code-patching.h
> index 4ba834599c4d..f3445188d319 100644
> --- a/arch/powerpc/include/asm/code-patching.h
> +++ b/arch/powerpc/include/asm/code-patching.h
> @@ -110,7 +110,7 @@ static inline unsigned long ppc_function_entry(void *func)
>* function's descriptor. The first entry in the descriptor is the
>* address of the function text.
>*/
> - return ((func_descr_t *)func)->entry;
> + return ((struct ppc64_opd_entry *)func)->addr;
> #else
> return (unsigned long)func;
> #endif
> diff --git a/arch/powerpc/include/asm/types.h
> b/arch/powerpc/include/asm/types.h
> index f1630c553efe..97da77bc48c9 100644
> --- a/arch/powerpc/include/asm/types.h
> +++ b/arch/powerpc/include/asm/types.h
> @@ -23,12 +23,6 @@
>
> typedef __vector128 vector128;
>
> -typedef struct {
> - unsigned long entry;
> - unsigned long toc;
> - unsigned long env;
> -} func_descr_t;
I was a little concerned about going from a 3-element struct to a
2-element struct (as ppc64_opd_entry doesn't have an element for env) -
but we don't seem to take the sizeof this anywhere, nor do we use env
anywhere, nor do we do funky macro stuff with it in the signal handling
code that might implictly use the 3rd element, so I guess this will
work. Still, func_descr_t seems to describe the underlying ABI better
than ppc64_opd_entry...
> #endif /* __ASSEMBLY__ */
>
> #endif /* _ASM_POWERPC_TYPES_H */
> diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
> index 1831bba0582e..63ddbe7b108c 100644
> --- a/arch/powerpc/kernel/signal_64.c
> +++ b/arch/powerpc/kernel/signal_64.c
> @@ -933,11 +933,11 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t
> *set,
>* descriptor is the entry address of signal and the second
>* entry is the TOC value we need to use.
>*/
> - func_descr_t __user *funct_desc_ptr =
> - (func_descr_t __user *) ksig->ka.sa.sa_handler;
> + struct ppc64_opd_entry __user *funct_desc_ptr =
> + (struct ppc64_opd_entry __user *)ksig->ka.sa.sa_handler;
>
> - err |= get_user(regs->ctr, _desc_ptr->entry);
> - err |= get_user(regs->gpr[2], _desc_ptr->toc);
> + err |= get_user(regs->ctr, _desc_ptr->addr);
> + err |= get_user(regs->gpr[2], _desc_ptr->r2);
Likewise, r2 seems like a worse name than toc. I guess we could clean
that up another time though.
Kind regards,
Daniel
> }
>
> /* enter the signal handler in native-endian mode */
> --
> 2.31.1
Re: [PATCH] powerpc/dcr: Use cmplwi instead of 3-argument cmpli
On Wed, Oct 13, 2021 at 7:44 PM Michael Ellerman wrote: > > In dcr-low.S we use cmpli with three arguments, instead of four > arguments as defined in the ISA: > > cmpli cr0,r3,1024 > > This appears to be a PPC440-ism, looking at the "PPC440x5 CPU Core > User’s Manual" it shows cmpli having no L field, but implied to be 0 due > to the core being 32-bit. It mentions that the ISA defines four > arguments and recommends using cmplwi. > > dcr-low.S is only built 32-bit, because it is only built when > DCR_NATIVE=y, which is only selected by 40x and 44x. Looking at the > generated code (with gcc/gas) we see cmplwi as expected. > > Although gas is happy with the 3-argument version when building for > 32-bit, the LLVM assembler is not and errors out with: > > arch/powerpc/sysdev/dcr-low.S:27:10: error: invalid operand for instruction >cmpli 0,%r3,1024; ... >^ > > Switching to the four argument version avoids any confusion when reading > the ISA, fixes the issue with the LLVM assembler, and also means the > code could be built 64-bit in future (though that's very unlikely). Thank you Michael. We've definitely run into a few cases where GAS allowed for various short-hand forms of various instructions (a fair amount of recent work was around 32b ARM and THUMB parity in LLVM). LLVM's assembler is mostly generated from a high level description of the instruction formats, so it's not always as flexible as a hand written parser would be. (There is a mix of hand written arch specific parsing, but most of the parser is arch agnostic, and all of the instruction descriptions are described in an LLVM specific high level language called tablegen which generates C++ that is used by the assembler, but also the disassembler, the compiler, and even the linker if need be). Link: https://github.com/ClangBuiltLinux/linux/issues/1419 Reviewed-by: Nick Desaulniers > Reported-by: Nick Desaulniers > Signed-off-by: Michael Ellerman > --- > arch/powerpc/sysdev/dcr-low.S | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/powerpc/sysdev/dcr-low.S b/arch/powerpc/sysdev/dcr-low.S > index efeeb1b885a1..329b9c4ae542 100644 > --- a/arch/powerpc/sysdev/dcr-low.S > +++ b/arch/powerpc/sysdev/dcr-low.S > @@ -11,7 +11,7 @@ > #include > > #define DCR_ACCESS_PROLOG(table) \ > - cmpli cr0,r3,1024; \ > + cmplwi cr0,r3,1024; \ > rlwinm r3,r3,4,18,27; \ > lis r5,table@h; \ > ori r5,r5,table@l; \ > -- > 2.25.1 > -- Thanks, ~Nick Desaulniers
Re: [PATCH v2] scsi: ibmvscsi: Use dma_alloc_noncoherent() instead of get_zeroed_page/dma_map_single()
On 10/11/21 8:23 PM, Cai Huoqing wrote:
> Replacing get_zeroed_page/free_page/dma_map_single/dma_unmap_single()
> with dma_alloc_noncoherent/dma_free_noncoherent() helps to reduce
> code size, and simplify the code, and the hardware can keeep DMA
> coherent itsel
Not sure why the switch from coherent in v1 to noncoherent in v2. I think that
was unnecessary and I believe requires explicit synchronization via
dma_sync_single_{for_device|for_cpu} calls.
Further, as both kernel-bot and Nathan have already pointed out this doesn't
even compile.
-Tyrel
>
> Signed-off-by: Cai Huoqing
> ---
> v1->v2:
> *Change to dma_alloc/free_noncoherent from dma_alloc/free_coherent.
> *Update changelog.
>
> drivers/scsi/ibmvscsi/ibmvfc.c | 16
> drivers/scsi/ibmvscsi/ibmvscsi.c | 29 +
> 2 files changed, 13 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
> index 1f1586ad48fe..6e95fd02fd25 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc.c
> @@ -869,8 +869,8 @@ static void ibmvfc_free_queue(struct ibmvfc_host *vhost,
> {
> struct device *dev = vhost->dev;
>
> - dma_unmap_single(dev, queue->msg_token, PAGE_SIZE, DMA_BIDIRECTIONAL);
> - free_page((unsigned long)queue->msgs.handle);
> + dma_free_noncoherent(dev, PAGE_SIZE, queue->msgs.handle,
> + queue->msg_token, DMA_BIDIRECTIONAL);
> queue->msgs.handle = NULL;
>
> ibmvfc_free_event_pool(vhost, queue);
> @@ -5663,19 +5663,11 @@ static int ibmvfc_alloc_queue(struct ibmvfc_host
> *vhost,
> return -ENOMEM;
> }
>
> - queue->msgs.handle = (void *)get_zeroed_page(GFP_KERNEL);
> + queue->msgs.handle = dma_alloc_noncoherent(dev, PAGE_SIZE,
> >msg_token,
> +DMA_BIDIRECTIONAL,
> GFP_KERNEL);
> if (!queue->msgs.handle)
> return -ENOMEM;
>
> - queue->msg_token = dma_map_single(dev, queue->msgs.handle, PAGE_SIZE,
> - DMA_BIDIRECTIONAL);
> -
> - if (dma_mapping_error(dev, queue->msg_token)) {
> - free_page((unsigned long)queue->msgs.handle);
> - queue->msgs.handle = NULL;
> - return -ENOMEM;
> - }
> -
> queue->cur = 0;
> queue->fmt = fmt;
> queue->size = PAGE_SIZE / fmt_size;
> diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c
> b/drivers/scsi/ibmvscsi/ibmvscsi.c
> index ea8e01f49cba..68409c298c74 100644
> --- a/drivers/scsi/ibmvscsi/ibmvscsi.c
> +++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
> @@ -151,10 +151,8 @@ static void ibmvscsi_release_crq_queue(struct crq_queue
> *queue,
> msleep(100);
> rc = plpar_hcall_norets(H_FREE_CRQ, vdev->unit_address);
> } while ((rc == H_BUSY) || (H_IS_LONG_BUSY(rc)));
> - dma_unmap_single(hostdata->dev,
> - queue->msg_token,
> - queue->size * sizeof(*queue->msgs), DMA_BIDIRECTIONAL);
> - free_page((unsigned long)queue->msgs);
> + dma_free_noncoherent(hostdata->dev, PAGE_SIZE,
> + queue->msgs, queue->msg_token, DMA_BIDIRECTIONAL);
> }
>
> /**
> @@ -331,18 +329,12 @@ static int ibmvscsi_init_crq_queue(struct crq_queue
> *queue,
> int retrc;
> struct vio_dev *vdev = to_vio_dev(hostdata->dev);
>
> - queue->msgs = (struct viosrp_crq *)get_zeroed_page(GFP_KERNEL);
> -
> - if (!queue->msgs)
> - goto malloc_failed;
> queue->size = PAGE_SIZE / sizeof(*queue->msgs);
> -
> - queue->msg_token = dma_map_single(hostdata->dev, queue->msgs,
> - queue->size * sizeof(*queue->msgs),
> - DMA_BIDIRECTIONAL);
> -
> - if (dma_mapping_error(hostdata->dev, queue->msg_token))
> - goto map_failed;
> + queue->msgs = dma_alloc_noncoherent(hostdata->dev,
> + PAGE_SIZE, >msg_token,
> + DMA_BIDIRECTIONAL, GFP_KERNEL);
> + if (!queue->msg)
> + goto malloc_failed;
>
> gather_partition_info();
> set_adapter_info(hostdata);
> @@ -395,11 +387,8 @@ static int ibmvscsi_init_crq_queue(struct crq_queue
> *queue,
> rc = plpar_hcall_norets(H_FREE_CRQ, vdev->unit_address);
> } while ((rc == H_BUSY) || (H_IS_LONG_BUSY(rc)));
>reg_crq_failed:
> - dma_unmap_single(hostdata->dev,
> - queue->msg_token,
> - queue->size * sizeof(*queue->msgs), DMA_BIDIRECTIONAL);
> - map_failed:
> - free_page((unsigned long)queue->msgs);
> + dma_free_noncoherent(hostdata->dev, PAGE_SIZE, queue->msg,
> + queue->msg_token, DMA_BIDIRECTIONAL);
>malloc_failed:
> return -1;
> }
>
Re: [PATCH v2 02/13] powerpc: Rename 'funcaddr' to 'addr' in 'struct ppc64_opd_entry'
Christophe Leroy writes:
> There are three architectures with function descriptors, try to
> have common names for the address they contain in order to
> refactor some functions into generic functions later.
>
> powerpc has 'funcaddr'
> ia64 has 'ip'
> parisc has 'addr'
>
> Vote for 'addr' and update 'struct ppc64_opd_entry' accordingly.
I would have picked 'funcaddr', but at least 'addr' is better than 'ip'!
And I agree that consistency, and then making things generic is worthwhile.
I grepped the latest powerpc/next for uses of 'funcaddr'. There were 5,
your patch changes all 5.
The series passes build tests and this patch has no checkpatch or other
style concerns.
On that basis:
Reviewed-by: Daniel Axtens
Kind regards,
Daniel
> Reviewed-by: Kees Cook
> Signed-off-by: Christophe Leroy
> ---
> arch/powerpc/include/asm/elf.h | 2 +-
> arch/powerpc/include/asm/sections.h | 2 +-
> arch/powerpc/kernel/module_64.c | 6 +++---
> 3 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/elf.h b/arch/powerpc/include/asm/elf.h
> index a4406714c060..bb0f278f9ed4 100644
> --- a/arch/powerpc/include/asm/elf.h
> +++ b/arch/powerpc/include/asm/elf.h
> @@ -178,7 +178,7 @@ void relocate(unsigned long final_address);
>
> /* There's actually a third entry here, but it's unused */
> struct ppc64_opd_entry {
> - unsigned long funcaddr;
> + unsigned long addr;
> unsigned long r2;
> };
>
> diff --git a/arch/powerpc/include/asm/sections.h
> b/arch/powerpc/include/asm/sections.h
> index 6e4af4492a14..32e7035863ac 100644
> --- a/arch/powerpc/include/asm/sections.h
> +++ b/arch/powerpc/include/asm/sections.h
> @@ -77,7 +77,7 @@ static inline void *dereference_function_descriptor(void
> *ptr)
> struct ppc64_opd_entry *desc = ptr;
> void *p;
>
> - if (!get_kernel_nofault(p, (void *)>funcaddr))
> + if (!get_kernel_nofault(p, (void *)>addr))
> ptr = p;
> return ptr;
> }
> diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
> index 6baa676e7cb6..82908c9be627 100644
> --- a/arch/powerpc/kernel/module_64.c
> +++ b/arch/powerpc/kernel/module_64.c
> @@ -72,11 +72,11 @@ static func_desc_t func_desc(unsigned long addr)
> }
> static unsigned long func_addr(unsigned long addr)
> {
> - return func_desc(addr).funcaddr;
> + return func_desc(addr).addr;
> }
> static unsigned long stub_func_addr(func_desc_t func)
> {
> - return func.funcaddr;
> + return func.addr;
> }
> static unsigned int local_entry_offset(const Elf64_Sym *sym)
> {
> @@ -187,7 +187,7 @@ static int relacmp(const void *_x, const void *_y)
> static unsigned long get_stubs_size(const Elf64_Ehdr *hdr,
> const Elf64_Shdr *sechdrs)
> {
> - /* One extra reloc so it's always 0-funcaddr terminated */
> + /* One extra reloc so it's always 0-addr terminated */
> unsigned long relocs = 1;
> unsigned i;
>
> --
> 2.31.1
Re: [PATCH v2 00/13] Fix LKDTM for PPC64/IA64/PARISC
Christophe Leroy writes: > PPC64/IA64/PARISC have function descriptors. LKDTM doesn't work > on those three architectures because LKDTM messes up function > descriptors with functions. Just to nitpick, it's powerpc 64-bit using the ELFv1 ABI. [1] The ELFv2 ABI [2] doesn't use function descriptors. (ELFv2 is used primarily for ppc64le, but some people like musl support it for BE as well.) This doesn't affect the correctness or desirability of your changes, it was just bugging me when I was reading the commit messages :-) Kind regards, Daniel [1] See e.g. https://refspecs.linuxfoundation.org/ELF/ppc64/PPC-elf64abi.html [2] https://openpowerfoundation.org/wp-content/uploads/2016/03/ABI64BitOpenPOWERv1.1_16July2015_pub4.pdf > This series does some cleanup in the three architectures and > refactors function descriptors so that it can then easily use it > in a generic way in LKDTM. > > Patch 8 is not absolutely necessary but it is a good trivial cleanup. > > Changes in v2: > - Addressed received comments > - Moved dereference_[kernel]_function_descriptor() out of line > - Added patches to remove func_descr_t and func_desc_t in powerpc > - Using func_desc_t instead of funct_descr_t > - Renamed HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR to HAVE_FUNCTION_DESCRIPTORS > - Added a new lkdtm test to check protection of function descriptors > > Christophe Leroy (13): > powerpc: Move 'struct ppc64_opd_entry' back into asm/elf.h > powerpc: Rename 'funcaddr' to 'addr' in 'struct ppc64_opd_entry' > powerpc: Remove func_descr_t > powerpc: Prepare func_desc_t for refactorisation > ia64: Rename 'ip' to 'addr' in 'struct fdesc' > asm-generic: Use HAVE_FUNCTION_DESCRIPTORS to define associated stubs > asm-generic: Define 'func_desc_t' to commonly describe function > descriptors > asm-generic: Refactor dereference_[kernel]_function_descriptor() > lkdtm: Force do_nothing() out of line > lkdtm: Really write into kernel text in WRITE_KERN > lkdtm: Fix lkdtm_EXEC_RODATA() > lkdtm: Fix execute_[user]_location() > lkdtm: Add a test for function descriptors protection > > arch/ia64/include/asm/elf.h | 2 +- > arch/ia64/include/asm/sections.h | 25 ++--- > arch/ia64/kernel/module.c| 6 +-- > arch/parisc/include/asm/sections.h | 17 +++--- > arch/parisc/kernel/process.c | 21 > arch/powerpc/include/asm/code-patching.h | 2 +- > arch/powerpc/include/asm/elf.h | 6 +++ > arch/powerpc/include/asm/sections.h | 30 ++- > arch/powerpc/include/asm/types.h | 6 --- > arch/powerpc/include/uapi/asm/elf.h | 8 --- > arch/powerpc/kernel/module_64.c | 38 + > arch/powerpc/kernel/signal_64.c | 8 +-- > drivers/misc/lkdtm/core.c| 1 + > drivers/misc/lkdtm/lkdtm.h | 1 + > drivers/misc/lkdtm/perms.c | 68 > include/asm-generic/sections.h | 13 - > include/linux/kallsyms.h | 2 +- > kernel/extable.c | 23 +++- > 18 files changed, 138 insertions(+), 139 deletions(-) > > -- > 2.31.1
Re: [PATCH v2 01/13] powerpc: Move 'struct ppc64_opd_entry' back into asm/elf.h
Hi Christophe,
> 'struct ppc64_opd_entry' doesn't belong to uapi/asm/elf.h
>
> It was initially in module_64.c and commit 2d291e902791 ("Fix compile
> failure with non modular builds") moved it into asm/elf.h
>
> But it was by mistake added outside of __KERNEL__ section,
> therefore commit c3617f72036c ("UAPI: (Scripted) Disintegrate
> arch/powerpc/include/asm") moved it to uapi/asm/elf.h
As Michael said on v1, I'm a little nervous about moving it out of uAPI
after so long, although I do take the points of Arnd and Kees that we're
not breaking compiled binaries, nor should people be using this struct
to begin with...
I've cc:ed the linux-api@ list.
Kind regards,
Daniel
> Move it back into asm/elf.h, this brings it back in line with
> IA64 and PARISC architectures.
>
> Fixes: 2d291e902791 ("Fix compile failure with non modular builds")
> Reviewed-by: Kees Cook
> Signed-off-by: Christophe Leroy
> ---
> arch/powerpc/include/asm/elf.h | 6 ++
> arch/powerpc/include/uapi/asm/elf.h | 8
> 2 files changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/elf.h b/arch/powerpc/include/asm/elf.h
> index b8425e3cfd81..a4406714c060 100644
> --- a/arch/powerpc/include/asm/elf.h
> +++ b/arch/powerpc/include/asm/elf.h
> @@ -176,4 +176,10 @@ do {
> \
> /* Relocate the kernel image to @final_address */
> void relocate(unsigned long final_address);
>
> +/* There's actually a third entry here, but it's unused */
> +struct ppc64_opd_entry {
> + unsigned long funcaddr;
> + unsigned long r2;
> +};
> +
> #endif /* _ASM_POWERPC_ELF_H */
> diff --git a/arch/powerpc/include/uapi/asm/elf.h
> b/arch/powerpc/include/uapi/asm/elf.h
> index 860c59291bfc..308857123a08 100644
> --- a/arch/powerpc/include/uapi/asm/elf.h
> +++ b/arch/powerpc/include/uapi/asm/elf.h
> @@ -289,12 +289,4 @@ typedef elf_fpreg_t elf_vsrreghalf_t32[ELF_NVSRHALFREG];
> /* Keep this the last entry. */
> #define R_PPC64_NUM 253
>
> -/* There's actually a third entry here, but it's unused */
> -struct ppc64_opd_entry
> -{
> - unsigned long funcaddr;
> - unsigned long r2;
> -};
> -
> -
> #endif /* _UAPI_ASM_POWERPC_ELF_H */
> --
> 2.31.1
Re: [PATCH v2] scsi: ibmvscsi: Use dma_alloc_noncoherent() instead of get_zeroed_page/dma_map_single()
Cai Huoqing writes: > @@ -331,18 +329,12 @@ static int ibmvscsi_init_crq_queue(struct crq_queue > *queue, > int retrc; > struct vio_dev *vdev = to_vio_dev(hostdata->dev); > > - queue->msgs = (struct viosrp_crq *)get_zeroed_page(GFP_KERNEL); > - > - if (!queue->msgs) > - goto malloc_failed; > queue->size = PAGE_SIZE / sizeof(*queue->msgs); > - > - queue->msg_token = dma_map_single(hostdata->dev, queue->msgs, > - queue->size * sizeof(*queue->msgs), > - DMA_BIDIRECTIONAL); > - > - if (dma_mapping_error(hostdata->dev, queue->msg_token)) > - goto map_failed; > + queue->msgs = dma_alloc_noncoherent(hostdata->dev, > + PAGE_SIZE, >msg_token, > + DMA_BIDIRECTIONAL, GFP_KERNEL); > + if (!queue->msg) > + goto malloc_failed; This version appears to retain the build breakage from v1 which was reported here: https://lore.kernel.org/linuxppc-dev/202110121452.nwphzezg-...@intel.com/ drivers/scsi/ibmvscsi/ibmvscsi.c: In function 'ibmvscsi_init_crq_queue': >> drivers/scsi/ibmvscsi/ibmvscsi.c:334:21: error: 'struct crq_queue' has no >> member named 'msg'; did you mean 'msgs'? 334 | if (!queue->msg) | ^~~ | msgs drivers/scsi/ibmvscsi/ibmvscsi.c:388:60: error: 'struct crq_queue' has no member named 'msg'; did you mean 'msgs'? 388 | dma_free_coherent(hostdata->dev, PAGE_SIZE, queue->msg, queue->msg_token); |^~~ |msgs
Re: [PATCH v3 1/2] ftrace: disable preemption between ftrace_test_recursion_trylock/unlock()
On Wed 2021-10-13 16:51:46, 王贇 wrote:
> As the documentation explained, ftrace_test_recursion_trylock()
> and ftrace_test_recursion_unlock() were supposed to disable and
> enable preemption properly, however currently this work is done
> outside of the function, which could be missing by mistake.
>
> This path will make sure the preemption was disabled when trylock()
> succeed, and the unlock() will enable the preemption if previously
> enabled.
>
> diff --git a/include/linux/trace_recursion.h b/include/linux/trace_recursion.h
> index a9f9c57..58e474c 100644
> --- a/include/linux/trace_recursion.h
> +++ b/include/linux/trace_recursion.h
> @@ -214,7 +214,18 @@ static __always_inline void trace_clear_recursion(int
> bit)
We should also update the description above the function, for example:
/**
* ftrace_test_recursion_trylock - tests for recursion in same context
*
* Use this for ftrace callbacks. This will detect if the function
* tracing recursed in the same context (normal vs interrupt),
*
* Returns: -1 if a recursion happened.
- * >= 0 if no recursion
+ * >= 0 if no recursion (success)
+ *
+ * Disables the preemption on success. It is just for a convenience.
+ * Current users needed to disable the preemtion for some reasons.
*/
> static __always_inline int ftrace_test_recursion_trylock(unsigned long ip,
>unsigned long
> parent_ip)
> {
> - return trace_test_and_set_recursion(ip, parent_ip, TRACE_FTRACE_START,
> TRACE_FTRACE_MAX);
> + int bit;
> +
> + bit = trace_test_and_set_recursion(ip, parent_ip, TRACE_FTRACE_START,
> TRACE_FTRACE_MAX);
> + /*
> + * The zero bit indicate we are nested
> + * in another trylock(), which means the
> + * preemption already disabled.
> + */
> + if (bit > 0)
> + preempt_disable_notrace();
Is this safe? The preemption is disabled only when
trace_test_and_set_recursion() was called by ftrace_test_recursion_trylock().
We must either always disable the preemtion when bit >= 0.
Or we have to disable the preemtion already in
trace_test_and_set_recursion().
Finally, the comment confused me a lot. The difference between nesting and
recursion is far from clear. And the code is tricky liky like hell :-)
I propose to add some comments, see below for a proposal.
> +
> + return bit;
> }
> /**
> @@ -222,9 +233,13 @@ static __always_inline int
> ftrace_test_recursion_trylock(unsigned long ip,
> * @bit: The return of a successful ftrace_test_recursion_trylock()
> *
> * This is used at the end of a ftrace callback.
> + *
> + * Preemption will be enabled (if it was previously enabled).
> */
> static __always_inline void ftrace_test_recursion_unlock(int bit)
> {
> + if (bit)
This is not symetric with trylock(). It should be:
if (bit > 0)
Anyway, trace_clear_recursion() quiently ignores bit != 0
> + preempt_enable_notrace();
> trace_clear_recursion(bit);
> }
Below is my proposed patch that tries to better explain the recursion
check:
>From 20d69f11e2683262fa0043b803999061cbac543f Mon Sep 17 00:00:00 2001
From: Petr Mladek
Date: Thu, 14 Oct 2021 16:57:39 +0200
Subject: [PATCH] trace: Better describe the recursion check return values
The trace recursion check might be called recursively by different
layers of the tracing code. It is safe recursion and the check
is even optimized for this case.
The problematic recursion is when the traced function is called
by the tracing code. This is properly detected.
Try to explain this difference a better way.
Signed-off-by: Petr Mladek
---
include/linux/trace_recursion.h | 16 +++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/include/linux/trace_recursion.h b/include/linux/trace_recursion.h
index a9f9c5714e65..b5efb804efdf 100644
--- a/include/linux/trace_recursion.h
+++ b/include/linux/trace_recursion.h
@@ -159,13 +159,27 @@ extern void ftrace_record_recursion(unsigned long ip,
unsigned long parent_ip);
# define do_ftrace_record_recursion(ip, pip) do { } while (0)
#endif
+/*
+ * trace_test_and_set_recursion() is called on several layers
+ * of the ftrace code when handling the same ftrace entry.
+ * These calls might be nested/recursive.
+ *
+ * It uses TRACE_LIST_*BITs to distinguish between this
+ * internal recursion and recursion caused by calling
+ * the traced function by the ftrace code.
+ *
+ * Returns: > 0 when no recursion
+ * 0 when called recursively internally (safe)
+ * -1 when the traced function was called recursively from
+ * the ftrace handler (unsafe)
+ */
static __always_inline int trace_test_and_set_recursion(unsigned long ip,
unsigned long pip,
int start, int max)
{
unsigned int val = READ_ONCE(current->trace_recursion);
int bit;
- /* A previous recursion
Re: [PATCH] powerpc/pseries/iommu: Add of_node_put() before break
Hello Wan, thank you for this patch.
On Thu, 2021-10-14 at 03:56 -0400, Wan Jiabing wrote:
> Fix following coccicheck warning:
>
> ./arch/powerpc/platforms/pseries/iommu.c:924:1-28: WARNING: Function
> for_each_node_with_property should have of_node_put() before break
>
> Early exits from for_each_node_with_property should decrement the
> node reference counter.
Yeah, it makes sense to me.
for_each_node_with_property calls of_find_node_with_property() at each
step, which ends up calling of_node_put() after using each node.
Introducing this break caused this of_node_put not to happen to the
last node, so IIUC this patch fixes a possible issue if kzalloc() fails
in ddw_list_new_entry().
Another option would be s/break/continue here, but it does not make
sense to keep trying next nodes if there is no memory to allocate for a
struct dma_win (4 pointers).
On the other hard, failing on allocating such small space should not
happen often (if it will ever happen), so a 'continue' here makes code
simpler.
Anyway, FWIW:
Reviewed-by: Leonardo Bras
Best regards,
Leo
>
> Signed-off-by: Wan Jiabing
> ---
> arch/powerpc/platforms/pseries/iommu.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/platforms/pseries/iommu.c
> b/arch/powerpc/platforms/pseries/iommu.c
> index 269f61d519c2..c140aa683f66 100644
> --- a/arch/powerpc/platforms/pseries/iommu.c
> +++ b/arch/powerpc/platforms/pseries/iommu.c
> @@ -929,8 +929,10 @@ static void find_existing_ddw_windows_named(const
> char *name)
> }
>
> window = ddw_list_new_entry(pdn, dma64);
> - if (!window)
> + if (!window) {
> + of_node_put(pdn);
> break;
> + }
>
> spin_lock(_win_list_lock);
> list_add(>list, _win_list);
Re: [PATCH v3 1/2] ftrace: disable preemption between ftrace_test_recursion_trylock/unlock()
On Thu, 14 Oct 2021 11:13:13 +0200 (CEST) Miroslav Benes wrote: > for the livepatch part of the patch. > > I would also ask you not to submit new versions so often, so that the > other reviewers have time to actually review the patch set. > > Quoting from Documentation/process/submitting-patches.rst: > > "Wait for a minimum of one week before resubmitting or pinging reviewers - > possibly longer during busy times like merge windows." Although, for updates on a small patch set, I would say just a couple of days, instead of a week. It's annoying when you have a 15 patch set series, that gets updated on a daily basis. But for something with only 2 patches, wait just two days. At least, that's how I feel. -- Steve
Re: [PATCH 1/5] dt-bindings: memory: fsl: convert ifc binding to yaml schema
On Mon, Oct 4, 2021 at 4:31 AM Krzysztof Kozlowski wrote: > > On 01/10/2021 18:17, Li Yang wrote: > > On Fri, Oct 1, 2021 at 5:01 AM Krzysztof Kozlowski > > wrote: > >> > > (...) > > >>> + > >>> + interrupts: > >>> +minItems: 1 > >>> +maxItems: 2 > >>> +description: | > >>> + IFC may have one or two interrupts. If two interrupt specifiers > >>> are > >>> + present, the first is the "common" interrupt (CM_EVTER_STAT), and > >>> the > >>> + second is the NAND interrupt (NAND_EVTER_STAT). If there is only > >>> one, > >>> + that interrupt reports both types of event. > >>> + > >>> + little-endian: > >>> +$ref: '/schemas/types.yaml#/definitions/flag' > >> > >> type: boolean > > > > It will not have a true or false value, but only present or not. Is > > the boolean type taking care of this too? > > boolean is for a property which does not accept values and true/false > depends on its presence. > See: > Documentation/devicetree/bindings/phy/lantiq,vrx200-pcie-phy.yaml > Documentation/devicetree/bindings/thermal/qoriq-thermal.yaml They are equivalent, so either can be used. Really what is needed here is a common schema for the endianness properties defining the type once. Then any binding using a property can just do 'little-endian: true'. Rob
Re: [PATCH] powerpc/mpc512x: dts: fix PSC node warnings
On Thu, Oct 14, 2021 at 6:31 AM Anatolij Gustschin wrote:
>
> Fix build warnings like:
> mpc5121.dtsi:397.13-406.5: Warning (spi_bus_bridge): /soc@8000/psc@11400:
> node name for SPI buses should be 'spi'
> mpc5121.dtsi:409.13-418.5: Warning (spi_bus_bridge): /soc@8000/psc@11500:
> node name for SPI buses should be 'spi'
> mpc5121.dtsi:457.13-466.5: Warning (spi_bus_bridge): /soc@8000/psc@11900:
> node name for SPI buses should be 'spi'
>
> Signed-off-by: Anatolij Gustschin
> ---
> arch/powerpc/boot/dts/ac14xx.dts | 17 +++--
> arch/powerpc/boot/dts/pdm360ng.dts | 11 ++-
> 2 files changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/boot/dts/ac14xx.dts
> b/arch/powerpc/boot/dts/ac14xx.dts
> index 5d8877e1f4ad..662d7aa2e4e8 100644
> --- a/arch/powerpc/boot/dts/ac14xx.dts
> +++ b/arch/powerpc/boot/dts/ac14xx.dts
> @@ -301,13 +301,21 @@
> fsl,tx-fifo-size = <512>;
> };
>
> + /delete-node/ psc@11400;
> + /delete-node/ psc@11500;
That's an odd way to fix this, and means every user of the .dtsi file
with these nodes will have to repeat the same thing.
> +
> /* PSC4 in SPI mode */
> - spi4: psc@11400 {
> + spi4: spi@11400 {
> compatible = "fsl,mpc5121-psc-spi", "fsl,mpc5121-psc";
> + reg = <0x11400 0x100>;
> fsl,rx-fifo-size = <768>;
> fsl,tx-fifo-size = <768>;
> #address-cells = <1>;
> #size-cells = <0>;
> + interrupts = <40 0x8>;
> + clocks = < MPC512x_CLK_PSC4>,
> +< MPC512x_CLK_PSC4_MCLK>;
> + clock-names = "ipg", "mclk";
> num-cs = <1>;
> cs-gpios = <_pic 25 0>;
>
> @@ -326,13 +334,18 @@
> };
>
> /* PSC5 in SPI mode */
> - spi5: psc@11500 {
> + spi5: spi@11500 {
> compatible = "fsl,mpc5121-psc-spi", "fsl,mpc5121-psc";
> + reg = <0x11500 0x100>;
> fsl,mode = "spi-master";
> fsl,rx-fifo-size = <128>;
> fsl,tx-fifo-size = <128>;
> #address-cells = <1>;
> #size-cells = <0>;
> + interrupts = <40 0x8>;
> + clocks = < MPC512x_CLK_PSC5>,
> +< MPC512x_CLK_PSC5_MCLK>;
> + clock-names = "ipg", "mclk";
>
> lcd@0 {
> compatible = "ilitek,ili922x";
> diff --git a/arch/powerpc/boot/dts/pdm360ng.dts
> b/arch/powerpc/boot/dts/pdm360ng.dts
> index 67c3b9db75d7..2733d15079a9 100644
> --- a/arch/powerpc/boot/dts/pdm360ng.dts
> +++ b/arch/powerpc/boot/dts/pdm360ng.dts
> @@ -169,10 +169,19 @@
> compatible = "fsl,mpc5121-psc-uart",
> "fsl,mpc5121-psc";
> };
>
> - psc@11900 {
> + /delete-node/ psc@11900;
> +
> + spi@11900 {
> compatible = "fsl,mpc5121-psc-spi", "fsl,mpc5121-psc";
> + reg = <0x11900 0x100>;
> #address-cells = <1>;
> #size-cells = <0>;
> + interrupts = <40 0x8>;
> + fsl,rx-fifo-size = <16>;
> + fsl,tx-fifo-size = <16>;
> + clocks = < MPC512x_CLK_PSC9>,
> +< MPC512x_CLK_PSC9_MCLK>;
> + clock-names = "ipg", "mclk";
>
> /* ADS7845 touch screen controller */
> ts@0 {
> --
> 2.17.1
>
Re: [RFC PATCH] powerpc: dts: Remove MPC5xxx platforms
On Wed, 13 Oct 2021 17:38:08 +1100 Stephen Rothwell s...@canb.auug.org.au wrote: >Hi Rob, > >On Tue, 12 Oct 2021 10:34:56 -0500 Rob Herring wrote: >> >> The mpc5xxx platforms have had dts warnings for some time which no one >> seems to care to fix, so let's just remove the dts files. >> >> According to Arnd: >> "Specifically, MPC5200B has a 15 year lifetime, which ends in >> 11 months from now. The original bplan/Genesi Efika 5K2 was >> quite popular at the time it came out, and there are probably >> still some of those hanging around, but they came with Open >> Firmware rather than relying on the dts files that ship with the >> kernel. >> >> Grant Likely was the original maintainer for MPC52xx until 2011, >> Anatolij Gustschin is still listed as maintainer since then but hasn't >> been active in it for a while either. Anatolij can probably best judge >> which of these boards are still in going to be used with future kernels, >> but I suspect once you start removing bits from 52xx, the newer >> but less common 512x platform can go away as well." >> >> Cc: Anatolij Gustschin >> Cc: Arnd Bergmann >> Cc: Stephen Rothwell >> Cc: Michael Ellerman >> Cc: Benjamin Herrenschmidt >> Cc: Paul Mackerras >> Cc: linuxppc-dev@lists.ozlabs.org >> Signed-off-by: Rob Herring >> --- >> Sending this out as a feeler to see if anyone cares. If anyone does, >> please fix the warnings. I've sent patches to fix the warnings. Thanks, Anatolij
Re: linux-next: build warnings in Linus' tree
On Thu, 14 Oct 2021 10:44:46 +0200 Arnd Bergmann a...@arndb.de wrote: >On Thu, Oct 14, 2021 at 12:12 AM Anatolij Gustschin wrote: >> On Tue, 12 Oct 2021 16:39:56 +0200 >> Arnd Bergmann a...@arndb.de wrote: >> ... >> >Grant Likely was the original maintainer for MPC52xx until 2011, >> >Anatolij Gustschin is still listed as maintainer since then but hasn't >> >been active in it for a while either. Anatolij can probably best judge >> >which of these boards are still in going to be used with future kernels, >> >but I suspect once you start removing bits from 52xx, the newer >> >but less common 512x platform can go away as well. >> >> many of these boards are still used, i.e. o2d*, digsy_mtc, tqm5200. > >Just for clarification, I assume when you say "still used" that implies >getting updated to new kernels rather than just running the old BSPs, >right? yes, at least some of them. I used v5.4 kernel on digsy_mtc and tqm5200 last year, and v5.10 kernel is also known to work. >What are the typical distro release cycles for those machines >you list: do you move from one LTS kernel to the next each year, >or are they getting more sporadic over time? these machines are in embedded systems and do not get regular distro updates, therefore more sporadic over time. >Do you expect the machines with the lowest memory such as the >32MB digsy to stop getting kernel updates before the others? No. There are also digsy variants with 256MiB DRAM. Thanks, Anatolij
[PATCH] powerpc/mpc512x: dts: fix PSC node warnings
Fix build warnings like:
mpc5121.dtsi:397.13-406.5: Warning (spi_bus_bridge): /soc@8000/psc@11400:
node name for SPI buses should be 'spi'
mpc5121.dtsi:409.13-418.5: Warning (spi_bus_bridge): /soc@8000/psc@11500:
node name for SPI buses should be 'spi'
mpc5121.dtsi:457.13-466.5: Warning (spi_bus_bridge): /soc@8000/psc@11900:
node name for SPI buses should be 'spi'
Signed-off-by: Anatolij Gustschin
---
arch/powerpc/boot/dts/ac14xx.dts | 17 +++--
arch/powerpc/boot/dts/pdm360ng.dts | 11 ++-
2 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/boot/dts/ac14xx.dts b/arch/powerpc/boot/dts/ac14xx.dts
index 5d8877e1f4ad..662d7aa2e4e8 100644
--- a/arch/powerpc/boot/dts/ac14xx.dts
+++ b/arch/powerpc/boot/dts/ac14xx.dts
@@ -301,13 +301,21 @@
fsl,tx-fifo-size = <512>;
};
+ /delete-node/ psc@11400;
+ /delete-node/ psc@11500;
+
/* PSC4 in SPI mode */
- spi4: psc@11400 {
+ spi4: spi@11400 {
compatible = "fsl,mpc5121-psc-spi", "fsl,mpc5121-psc";
+ reg = <0x11400 0x100>;
fsl,rx-fifo-size = <768>;
fsl,tx-fifo-size = <768>;
#address-cells = <1>;
#size-cells = <0>;
+ interrupts = <40 0x8>;
+ clocks = < MPC512x_CLK_PSC4>,
+< MPC512x_CLK_PSC4_MCLK>;
+ clock-names = "ipg", "mclk";
num-cs = <1>;
cs-gpios = <_pic 25 0>;
@@ -326,13 +334,18 @@
};
/* PSC5 in SPI mode */
- spi5: psc@11500 {
+ spi5: spi@11500 {
compatible = "fsl,mpc5121-psc-spi", "fsl,mpc5121-psc";
+ reg = <0x11500 0x100>;
fsl,mode = "spi-master";
fsl,rx-fifo-size = <128>;
fsl,tx-fifo-size = <128>;
#address-cells = <1>;
#size-cells = <0>;
+ interrupts = <40 0x8>;
+ clocks = < MPC512x_CLK_PSC5>,
+< MPC512x_CLK_PSC5_MCLK>;
+ clock-names = "ipg", "mclk";
lcd@0 {
compatible = "ilitek,ili922x";
diff --git a/arch/powerpc/boot/dts/pdm360ng.dts
b/arch/powerpc/boot/dts/pdm360ng.dts
index 67c3b9db75d7..2733d15079a9 100644
--- a/arch/powerpc/boot/dts/pdm360ng.dts
+++ b/arch/powerpc/boot/dts/pdm360ng.dts
@@ -169,10 +169,19 @@
compatible = "fsl,mpc5121-psc-uart", "fsl,mpc5121-psc";
};
- psc@11900 {
+ /delete-node/ psc@11900;
+
+ spi@11900 {
compatible = "fsl,mpc5121-psc-spi", "fsl,mpc5121-psc";
+ reg = <0x11900 0x100>;
#address-cells = <1>;
#size-cells = <0>;
+ interrupts = <40 0x8>;
+ fsl,rx-fifo-size = <16>;
+ fsl,tx-fifo-size = <16>;
+ clocks = < MPC512x_CLK_PSC9>,
+< MPC512x_CLK_PSC9_MCLK>;
+ clock-names = "ipg", "mclk";
/* ADS7845 touch screen controller */
ts@0 {
--
2.17.1
Re: [PATCH] powerpc/dcr: Use cmplwi instead of 3-argument cmpli
Hi! On Thu, Oct 14, 2021 at 01:44:24PM +1100, Michael Ellerman wrote: > In dcr-low.S we use cmpli with three arguments, instead of four > arguments as defined in the ISA: > > cmpli cr0,r3,1024 > > This appears to be a PPC440-ism, looking at the "PPC440x5 CPU Core > User’s Manual" it shows cmpli having no L field, but implied to be 0 due > to the core being 32-bit. It mentions that the ISA defines four > arguments and recommends using cmplwi. It also corresponds to the old POWER instruction set, which had no L field there, a reserved bit instead. It used to be that -many allowed these insns as well, but not anymore. > Although gas is happy with the 3-argument version when building for > 32-bit, the LLVM assembler is not and errors out with: A GAS targeting powerpc64 isn't happy either, fwiw. > arch/powerpc/sysdev/dcr-low.S:27:10: error: invalid operand for instruction >cmpli 0,%r3,1024; ... >^ > > Switching to the four argument version avoids any confusion when reading > the ISA, fixes the issue with the LLVM assembler, and also means the > code could be built 64-bit in future (though that's very unlikely). You are actually now using to the extended opcode cmpwli (a much better plan :-) ) Thanks, Segher
RE: [RESEND PATCH v4 0/8] bpf powerpc: Add BPF_PROBE_MEM support in powerpc JIT compiler
From: Christophe Leroy
> Sent: 14 October 2021 09:34
>
> Le 14/10/2021 à 10:15, David Laight a écrit :
> > From: Hari Bathini
> >> Sent: 12 October 2021 13:31
> >>
> >> Patch #1 & #2 are simple cleanup patches. Patch #3 refactors JIT
> >> compiler code with the aim to simplify adding BPF_PROBE_MEM support.
> >> Patch #4 introduces PPC_RAW_BRANCH() macro instead of open coding
> >> branch instruction. Patch #5 & #7 add BPF_PROBE_MEM support for PPC64
> >> & PPC32 JIT compilers respectively. Patch #6 & #8 handle bad userspace
> >> pointers for PPC64 & PPC32 cases respectively.
> >
> > I thought that BPF was only allowed to do fairly restricted
> > memory accesses - so WTF does it need a BPF_PROBE_MEM instruction?
> >
>
>
> Looks like it's been added by commit 2a02759ef5f8 ("bpf: Add support for
> BTF pointers to interpreter")
>
> They say in the log:
>
> Pointer to BTF object is a pointer to kernel object or NULL.
> The memory access in the interpreter has to be done via
> probe_kernel_read to avoid page faults.
Hmmm
Either the pointer should be valid (if not NULL) or they should
verify that it is the address of an interpreter.
If the value is being passed to/from userspace then they
are leaking kernel address - and that needs to be squashed.
They should be using an opaque identifier for the interpreter.
My gut feeling is that a lot of the changes to bpf over the last
few years means that it is no longer a verifiably safe simple
filter engine.
As such the you might as well load a normal kernel module.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT,
UK
Registration No: 1397386 (Wales)
[PATCH] dmaengine: bestcomm: fix system boot lockups
memset() and memcpy() on an MMIO region like here results in a
lockup at startup on mpc5200 platform (since this first happens
during probing of the ATA and Ethernet drivers). Use memset_io()
and memcpy_toio() instead.
Fixes: 2f9ea1bde0d1 ("bestcomm: core bestcomm support for Freescale MPC5200")
Cc: sta...@vger.kernel.org # v5.14+
Signed-off-by: Anatolij Gustschin
---
drivers/dma/bestcomm/ata.c | 2 +-
drivers/dma/bestcomm/bestcomm.c | 22 +++---
drivers/dma/bestcomm/fec.c | 4 ++--
drivers/dma/bestcomm/gen_bd.c | 4 ++--
4 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/dma/bestcomm/ata.c b/drivers/dma/bestcomm/ata.c
index 2fd87f83cf90..e169f18da551 100644
--- a/drivers/dma/bestcomm/ata.c
+++ b/drivers/dma/bestcomm/ata.c
@@ -133,7 +133,7 @@ void bcom_ata_reset_bd(struct bcom_task *tsk)
struct bcom_ata_var *var;
/* Reset all BD */
- memset(tsk->bd, 0x00, tsk->num_bd * tsk->bd_size);
+ memset_io(tsk->bd, 0x00, tsk->num_bd * tsk->bd_size);
tsk->index = 0;
tsk->outdex = 0;
diff --git a/drivers/dma/bestcomm/bestcomm.c b/drivers/dma/bestcomm/bestcomm.c
index d91cbbe7a48f..8c42e5ca00a9 100644
--- a/drivers/dma/bestcomm/bestcomm.c
+++ b/drivers/dma/bestcomm/bestcomm.c
@@ -95,7 +95,7 @@ bcom_task_alloc(int bd_count, int bd_size, int priv_size)
tsk->bd = bcom_sram_alloc(bd_count * bd_size, 4, >bd_pa);
if (!tsk->bd)
goto error;
- memset(tsk->bd, 0x00, bd_count * bd_size);
+ memset_io(tsk->bd, 0x00, bd_count * bd_size);
tsk->num_bd = bd_count;
tsk->bd_size = bd_size;
@@ -186,16 +186,16 @@ bcom_load_image(int task, u32 *task_image)
inc = bcom_task_inc(task);
/* Clear & copy */
- memset(var, 0x00, BCOM_VAR_SIZE);
- memset(inc, 0x00, BCOM_INC_SIZE);
+ memset_io(var, 0x00, BCOM_VAR_SIZE);
+ memset_io(inc, 0x00, BCOM_INC_SIZE);
desc_src = (u32 *)(hdr + 1);
var_src = desc_src + hdr->desc_size;
inc_src = var_src + hdr->var_size;
- memcpy(desc, desc_src, hdr->desc_size * sizeof(u32));
- memcpy(var + hdr->first_var, var_src, hdr->var_size * sizeof(u32));
- memcpy(inc, inc_src, hdr->inc_size * sizeof(u32));
+ memcpy_toio(desc, desc_src, hdr->desc_size * sizeof(u32));
+ memcpy_toio(var + hdr->first_var, var_src, hdr->var_size * sizeof(u32));
+ memcpy_toio(inc, inc_src, hdr->inc_size * sizeof(u32));
return 0;
}
@@ -302,13 +302,13 @@ static int bcom_engine_init(void)
return -ENOMEM;
}
- memset(bcom_eng->tdt, 0x00, tdt_size);
- memset(bcom_eng->ctx, 0x00, ctx_size);
- memset(bcom_eng->var, 0x00, var_size);
- memset(bcom_eng->fdt, 0x00, fdt_size);
+ memset_io(bcom_eng->tdt, 0x00, tdt_size);
+ memset_io(bcom_eng->ctx, 0x00, ctx_size);
+ memset_io(bcom_eng->var, 0x00, var_size);
+ memset_io(bcom_eng->fdt, 0x00, fdt_size);
/* Copy the FDT for the EU#3 */
- memcpy(_eng->fdt[48], fdt_ops, sizeof(fdt_ops));
+ memcpy_toio(_eng->fdt[48], fdt_ops, sizeof(fdt_ops));
/* Initialize Task base structure */
for (task=0; taskindex = 0;
tsk->outdex = 0;
- memset(tsk->bd, 0x00, tsk->num_bd * tsk->bd_size);
+ memset_io(tsk->bd, 0x00, tsk->num_bd * tsk->bd_size);
/* Configure some stuff */
bcom_set_task_pragma(tsk->tasknum, BCOM_FEC_RX_BD_PRAGMA);
@@ -241,7 +241,7 @@ bcom_fec_tx_reset(struct bcom_task *tsk)
tsk->index = 0;
tsk->outdex = 0;
- memset(tsk->bd, 0x00, tsk->num_bd * tsk->bd_size);
+ memset_io(tsk->bd, 0x00, tsk->num_bd * tsk->bd_size);
/* Configure some stuff */
bcom_set_task_pragma(tsk->tasknum, BCOM_FEC_TX_BD_PRAGMA);
diff --git a/drivers/dma/bestcomm/gen_bd.c b/drivers/dma/bestcomm/gen_bd.c
index 906ddba6a6f5..8a24a5cbc263 100644
--- a/drivers/dma/bestcomm/gen_bd.c
+++ b/drivers/dma/bestcomm/gen_bd.c
@@ -142,7 +142,7 @@ bcom_gen_bd_rx_reset(struct bcom_task *tsk)
tsk->index = 0;
tsk->outdex = 0;
- memset(tsk->bd, 0x00, tsk->num_bd * tsk->bd_size);
+ memset_io(tsk->bd, 0x00, tsk->num_bd * tsk->bd_size);
/* Configure some stuff */
bcom_set_task_pragma(tsk->tasknum, BCOM_GEN_RX_BD_PRAGMA);
@@ -226,7 +226,7 @@ bcom_gen_bd_tx_reset(struct bcom_task *tsk)
tsk->index = 0;
tsk->outdex = 0;
- memset(tsk->bd, 0x00, tsk->num_bd * tsk->bd_size);
+ memset_io(tsk->bd, 0x00, tsk->num_bd * tsk->bd_size);
/* Configure some stuff */
bcom_set_task_pragma(tsk->tasknum, BCOM_GEN_TX_BD_PRAGMA);
--
2.17.1
Re: [PATCH v3 1/2] ftrace: disable preemption between ftrace_test_recursion_trylock/unlock()
On 2021/10/14 下午5:13, Miroslav Benes wrote: >> diff --git a/kernel/livepatch/patch.c b/kernel/livepatch/patch.c >> index e8029ae..b8d75fb 100644 >> --- a/kernel/livepatch/patch.c >> +++ b/kernel/livepatch/patch.c >> @@ -49,14 +49,16 @@ static void notrace klp_ftrace_handler(unsigned long ip, >> >> ops = container_of(fops, struct klp_ops, fops); >> >> +/* >> + * > > This empty line is not useful. > >> + * The ftrace_test_recursion_trylock() will disable preemption, >> + * which is required for the variant of synchronize_rcu() that is >> + * used to allow patching functions where RCU is not watching. >> + * See klp_synchronize_transition() for more details. >> + */ >> bit = ftrace_test_recursion_trylock(ip, parent_ip); >> if (WARN_ON_ONCE(bit < 0)) >> return; >> -/* >> - * A variant of synchronize_rcu() is used to allow patching functions >> - * where RCU is not watching, see klp_synchronize_transition(). >> - */ >> -preempt_disable_notrace(); >> >> func = list_first_or_null_rcu(>func_stack, struct klp_func, >>stack_node); >> @@ -120,7 +122,6 @@ static void notrace klp_ftrace_handler(unsigned long ip, >> klp_arch_set_pc(fregs, (unsigned long)func->new_func); >> >> unlock: >> -preempt_enable_notrace(); >> ftrace_test_recursion_unlock(bit); >> } > > Acked-by: Miroslav Benes > > for the livepatch part of the patch. > > I would also ask you not to submit new versions so often, so that the > other reviewers have time to actually review the patch set. > > Quoting from Documentation/process/submitting-patches.rst: > > "Wait for a minimum of one week before resubmitting or pinging reviewers - > possibly longer during busy times like merge windows." Thanks for the Ack and suggestion, will take care from now on :-) Regards, Michael Wang > > Thanks > > Miroslav >
Re: [PATCH v3 1/2] ftrace: disable preemption between ftrace_test_recursion_trylock/unlock()
> diff --git a/kernel/livepatch/patch.c b/kernel/livepatch/patch.c > index e8029ae..b8d75fb 100644 > --- a/kernel/livepatch/patch.c > +++ b/kernel/livepatch/patch.c > @@ -49,14 +49,16 @@ static void notrace klp_ftrace_handler(unsigned long ip, > > ops = container_of(fops, struct klp_ops, fops); > > + /* > + * This empty line is not useful. > + * The ftrace_test_recursion_trylock() will disable preemption, > + * which is required for the variant of synchronize_rcu() that is > + * used to allow patching functions where RCU is not watching. > + * See klp_synchronize_transition() for more details. > + */ > bit = ftrace_test_recursion_trylock(ip, parent_ip); > if (WARN_ON_ONCE(bit < 0)) > return; > - /* > - * A variant of synchronize_rcu() is used to allow patching functions > - * where RCU is not watching, see klp_synchronize_transition(). > - */ > - preempt_disable_notrace(); > > func = list_first_or_null_rcu(>func_stack, struct klp_func, > stack_node); > @@ -120,7 +122,6 @@ static void notrace klp_ftrace_handler(unsigned long ip, > klp_arch_set_pc(fregs, (unsigned long)func->new_func); > > unlock: > - preempt_enable_notrace(); > ftrace_test_recursion_unlock(bit); > } Acked-by: Miroslav Benes for the livepatch part of the patch. I would also ask you not to submit new versions so often, so that the other reviewers have time to actually review the patch set. Quoting from Documentation/process/submitting-patches.rst: "Wait for a minimum of one week before resubmitting or pinging reviewers - possibly longer during busy times like merge windows." Thanks Miroslav
Re: [PATCH v10 2/3] tty: hvc: pass DMA capable memory to put_chars()
在 2021/10/14 下午4:41, Greg KH 写道:
On Thu, Oct 14, 2021 at 04:34:59PM +0800, Xianting Tian wrote:
在 2021/10/10 下午1:33, Greg KH 写道:
On Sat, Oct 09, 2021 at 11:45:23PM +0800, Xianting Tian wrote:
在 2021/10/9 下午7:58, Greg KH 写道:
Did you look at the placement using pahole as to how this structure now
looks?
thanks for all your commnts. for this one, do you mean I need to remove the
blank line? thanks
No, I mean to use the tool 'pahole' to see the structure layout that you
just created and determine if it really is the best way to add these new
fields, especially as you are adding huge buffers with odd alignment.
thanks,
Based on your comments, I removed 'char outchar', remian the position of
'int outbuf_size' unchanged to keep outbuf_size and lock in the same cache
line. Now hvc_struct change as below,
struct hvc_struct {
struct tty_port port;
spinlock_t lock;
int index;
int do_wakeup;
- char *outbuf;
int outbuf_size;
int n_outbuf;
uint32_t vtermno;
@@ -48,6 +57,16 @@ struct hvc_struct {
struct work_struct tty_resize;
struct list_head next;
unsigned long flags;
+
+ /*
+ * the buf is used in hvc console api for putting chars,
+ * and also used in hvc_poll_put_char() for putting single char.
+ */
+ char cons_outbuf[N_OUTBUF] __ALIGNED__;
+ spinlock_t cons_outbuf_lock;
+
+ /* the buf is used for putting chars to tty */
+ char outbuf[] __ALIGNED__;
};
pahole for above hvc_struct as below, is it ok for you? do we need to pack
the hole? thanks
struct hvc_struct {
struct tty_port port; /* 0 352 */
/* --- cacheline 5 boundary (320 bytes) was 32 bytes ago --- */
spinlock_t lock; /* 352 4 */
int index; /* 356 4 */
int do_wakeup; /* 360 4 */
int outbuf_size; /* 364 4 */
int n_outbuf; /* 368 4 */
uint32_t vtermno; /* 372 4 */
const struct hv_ops * ops; /* 376 8 */
/* --- cacheline 6 boundary (384 bytes) --- */
int irq_requested; /* 384 4 */
int data; /* 388 4 */
struct winsize ws; /* 392 8 */
struct work_struct tty_resize; /* 400 32 */
struct list_head next; /* 432 16 */
/* --- cacheline 7 boundary (448 bytes) --- */
long unsigned int flags; /* 448 8 */
/* XXX 56 bytes hole, try to pack */
/* --- cacheline 8 boundary (512 bytes) --- */
char cons_outbuf[16]; /* 512 16 */
spinlock_t cons_outbuf_lock; /* 528 4 */
/* XXX 44 bytes hole, try to pack */
Why not move the spinlock up above the cons_outbuf? Will that not be a
bit better?
thanks, I will move it avove cons_outbuf, and send v11 patches soon.
Anyway, this is all fine, and much better than before, thanks.
greg k-h
Re: [PATCH v2] powerpc/s64: Clarify that radix lacks DEBUG_PAGEALLOC
Le 13/10/2021 à 23:34, Joel Stanley a écrit :
The page_alloc.c code will call into __kernel_map_pages when
DEBUG_PAGEALLOC is configured and enabled.
As the implementation assumes hash, this should crash spectacularly if
not for a bit of luck in __kernel_map_pages. In this function
linear_map_hash_count is always zero, the for loop exits without doing
any damage.
There are no other platforms that determine if they support
debug_pagealloc at runtime. Instead of adding code to mm/page_alloc.c to
do that, this change turns the map/unmap into a noop when in radix
mode and prints a warning once.
Signed-off-by: Joel Stanley
---
v2: Put __kernel_map_pages in pgtable.h
arch/powerpc/include/asm/book3s/64/hash.h| 2 ++
arch/powerpc/include/asm/book3s/64/pgtable.h | 11 +++
arch/powerpc/include/asm/book3s/64/radix.h | 3 +++
arch/powerpc/mm/book3s64/hash_utils.c| 2 +-
arch/powerpc/mm/book3s64/radix_pgtable.c | 7 +++
5 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/book3s/64/hash.h
b/arch/powerpc/include/asm/book3s/64/hash.h
index d959b0195ad9..674fe0e890dc 100644
--- a/arch/powerpc/include/asm/book3s/64/hash.h
+++ b/arch/powerpc/include/asm/book3s/64/hash.h
@@ -255,6 +255,8 @@ int hash__create_section_mapping(unsigned long start,
unsigned long end,
int nid, pgprot_t prot);
int hash__remove_section_mapping(unsigned long start, unsigned long end);
+void hash__kernel_map_pages(struct page *page, int numpages, int enable);
+
#endif /* !__ASSEMBLY__ */
#endif /* __KERNEL__ */
#endif /* _ASM_POWERPC_BOOK3S_64_HASH_H */
diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h
b/arch/powerpc/include/asm/book3s/64/pgtable.h
index 5d34a8646f08..265661ded238 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -1101,6 +1101,17 @@ static inline void vmemmap_remove_mapping(unsigned long
start,
}
#endif
+#ifdef CONFIG_DEBUG_PAGEALLOC
+static inline void __kernel_map_pages(struct page *page, int numpages, int
enable)
+{
+ if (radix_enabled()) {
+ radix__kernel_map_pages(page, numpages, enable);
+ return;
+ }
+ hash__kernel_map_pages(page, numpages, enable);
I'd have prefered something like below
if (radix_enabled())
radix__kernel_map_pages(page, numpages, enable);
else
hash__kernel_map_pages(page, numpages, enable);
But regardless,
Reviewed-by: Christophe Leroy
+}
+#endif
+
static inline pte_t pmd_pte(pmd_t pmd)
{
return __pte_raw(pmd_raw(pmd));
diff --git a/arch/powerpc/include/asm/book3s/64/radix.h
b/arch/powerpc/include/asm/book3s/64/radix.h
index 59cab558e2f0..d090d9612348 100644
--- a/arch/powerpc/include/asm/book3s/64/radix.h
+++ b/arch/powerpc/include/asm/book3s/64/radix.h
@@ -316,5 +316,8 @@ int radix__create_section_mapping(unsigned long start,
unsigned long end,
int nid, pgprot_t prot);
int radix__remove_section_mapping(unsigned long start, unsigned long end);
#endif /* CONFIG_MEMORY_HOTPLUG */
+
+void radix__kernel_map_pages(struct page *page, int numpages, int enable);
+
#endif /* __ASSEMBLY__ */
#endif
diff --git a/arch/powerpc/mm/book3s64/hash_utils.c
b/arch/powerpc/mm/book3s64/hash_utils.c
index c145776d3ae5..cfd45245d009 100644
--- a/arch/powerpc/mm/book3s64/hash_utils.c
+++ b/arch/powerpc/mm/book3s64/hash_utils.c
@@ -1988,7 +1988,7 @@ static void kernel_unmap_linear_page(unsigned long vaddr,
unsigned long lmi)
mmu_kernel_ssize, 0);
}
-void __kernel_map_pages(struct page *page, int numpages, int enable)
+void hash__kernel_map_pages(struct page *page, int numpages, int enable)
{
unsigned long flags, vaddr, lmi;
int i;
diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c
b/arch/powerpc/mm/book3s64/radix_pgtable.c
index ae20add7954a..83b33418ad28 100644
--- a/arch/powerpc/mm/book3s64/radix_pgtable.c
+++ b/arch/powerpc/mm/book3s64/radix_pgtable.c
@@ -920,6 +920,13 @@ void __meminit radix__vmemmap_remove_mapping(unsigned long
start, unsigned long
#endif
#endif
+#ifdef CONFIG_DEBUG_PAGEALLOC
+void radix__kernel_map_pages(struct page *page, int numpages, int enable)
+{
+pr_warn_once("DEBUG_PAGEALLOC not supported in radix mode\n");
+}
+#endif
+
#ifdef CONFIG_TRANSPARENT_HUGEPAGE
unsigned long radix__pmd_hugepage_update(struct mm_struct *mm, unsigned long addr,
Re: linux-next: build warnings in Linus' tree
On Thu, Oct 14, 2021 at 12:12 AM Anatolij Gustschin wrote: > On Tue, 12 Oct 2021 16:39:56 +0200 > Arnd Bergmann a...@arndb.de wrote: > ... > >Grant Likely was the original maintainer for MPC52xx until 2011, > >Anatolij Gustschin is still listed as maintainer since then but hasn't > >been active in it for a while either. Anatolij can probably best judge > >which of these boards are still in going to be used with future kernels, > >but I suspect once you start removing bits from 52xx, the newer > >but less common 512x platform can go away as well. > > many of these boards are still used, i.e. o2d*, digsy_mtc, tqm5200. Just for clarification, I assume when you say "still used" that implies getting updated to new kernels rather than just running the old BSPs, right? What are the typical distro release cycles for those machines you list: do you move from one LTS kernel to the next each year, or are they getting more sporadic over time? Do you expect the machines with the lowest memory such as the 32MB digsy to stop getting kernel updates before the others? > I've sent first series to fix some warnings. Other dts fixes > require driver changes, so it will take some time to fix them. Thanks! Arnd
Re: [PATCH v10 2/3] tty: hvc: pass DMA capable memory to put_chars()
On Thu, Oct 14, 2021 at 04:34:59PM +0800, Xianting Tian wrote:
>
> 在 2021/10/10 下午1:33, Greg KH 写道:
> > On Sat, Oct 09, 2021 at 11:45:23PM +0800, Xianting Tian wrote:
> > > 在 2021/10/9 下午7:58, Greg KH 写道:
> > > > Did you look at the placement using pahole as to how this structure now
> > > > looks?
> > > thanks for all your commnts. for this one, do you mean I need to remove
> > > the
> > > blank line? thanks
> > >
> > No, I mean to use the tool 'pahole' to see the structure layout that you
> > just created and determine if it really is the best way to add these new
> > fields, especially as you are adding huge buffers with odd alignment.
>
> thanks,
>
> Based on your comments, I removed 'char outchar', remian the position of
> 'int outbuf_size' unchanged to keep outbuf_size and lock in the same cache
> line. Now hvc_struct change as below,
>
> struct hvc_struct {
> struct tty_port port;
> spinlock_t lock;
> int index;
> int do_wakeup;
> - char *outbuf;
> int outbuf_size;
> int n_outbuf;
> uint32_t vtermno;
> @@ -48,6 +57,16 @@ struct hvc_struct {
> struct work_struct tty_resize;
> struct list_head next;
> unsigned long flags;
> +
> + /*
> + * the buf is used in hvc console api for putting chars,
> + * and also used in hvc_poll_put_char() for putting single char.
> + */
> + char cons_outbuf[N_OUTBUF] __ALIGNED__;
> + spinlock_t cons_outbuf_lock;
> +
> + /* the buf is used for putting chars to tty */
> + char outbuf[] __ALIGNED__;
> };
>
> pahole for above hvc_struct as below, is it ok for you? do we need to pack
> the hole? thanks
>
> struct hvc_struct {
> struct tty_port port; /* 0 352 */
> /* --- cacheline 5 boundary (320 bytes) was 32 bytes ago --- */
> spinlock_t lock; /* 352 4 */
> int index; /* 356 4 */
> int do_wakeup; /* 360 4 */
> int outbuf_size; /* 364 4 */
> int n_outbuf; /* 368 4 */
> uint32_t vtermno; /* 372 4 */
> const struct hv_ops * ops; /* 376 8 */
> /* --- cacheline 6 boundary (384 bytes) --- */
> int irq_requested; /* 384 4 */
> int data; /* 388 4 */
> struct winsize ws; /* 392 8 */
> struct work_struct tty_resize; /* 400 32 */
> struct list_head next; /* 432 16 */
> /* --- cacheline 7 boundary (448 bytes) --- */
> long unsigned int flags; /* 448 8 */
>
> /* XXX 56 bytes hole, try to pack */
>
> /* --- cacheline 8 boundary (512 bytes) --- */
> char cons_outbuf[16]; /* 512 16 */
> spinlock_t cons_outbuf_lock; /* 528 4 */
>
> /* XXX 44 bytes hole, try to pack */
Why not move the spinlock up above the cons_outbuf? Will that not be a
bit better?
Anyway, this is all fine, and much better than before, thanks.
greg k-h
Re: [PATCH v10 2/3] tty: hvc: pass DMA capable memory to put_chars()
在 2021/10/10 下午1:33, Greg KH 写道:
On Sat, Oct 09, 2021 at 11:45:23PM +0800, Xianting Tian wrote:
在 2021/10/9 下午7:58, Greg KH 写道:
Did you look at the placement using pahole as to how this structure now
looks?
thanks for all your commnts. for this one, do you mean I need to remove the
blank line? thanks
No, I mean to use the tool 'pahole' to see the structure layout that you
just created and determine if it really is the best way to add these new
fields, especially as you are adding huge buffers with odd alignment.
thanks,
Based on your comments, I removed 'char outchar', remian the position
of 'int outbuf_size' unchanged to keep outbuf_size and lock in the same
cache line. Now hvc_struct change as below,
struct hvc_struct {
struct tty_port port;
spinlock_t lock;
int index;
int do_wakeup;
- char *outbuf;
int outbuf_size;
int n_outbuf;
uint32_t vtermno;
@@ -48,6 +57,16 @@ struct hvc_struct {
struct work_struct tty_resize;
struct list_head next;
unsigned long flags;
+
+ /*
+ * the buf is used in hvc console api for putting chars,
+ * and also used in hvc_poll_put_char() for putting single char.
+ */
+ char cons_outbuf[N_OUTBUF] __ALIGNED__;
+ spinlock_t cons_outbuf_lock;
+
+ /* the buf is used for putting chars to tty */
+ char outbuf[] __ALIGNED__;
};
pahole for above hvc_struct as below, is it ok for you? do we need to
pack the hole? thanks
struct hvc_struct {
struct tty_port port; /* 0 352 */
/* --- cacheline 5 boundary (320 bytes) was 32 bytes ago --- */
spinlock_t lock; /* 352 4 */
int index; /* 356 4 */
int do_wakeup; /* 360 4 */
int outbuf_size; /* 364 4 */
int n_outbuf; /* 368 4 */
uint32_t vtermno; /* 372 4 */
const struct hv_ops * ops; /* 376 8 */
/* --- cacheline 6 boundary (384 bytes) --- */
int irq_requested; /* 384 4 */
int data; /* 388 4 */
struct winsize ws; /* 392 8 */
struct work_struct tty_resize; /* 400 32 */
struct list_head next; /* 432 16 */
/* --- cacheline 7 boundary (448 bytes) --- */
long unsigned int flags; /* 448 8 */
/* XXX 56 bytes hole, try to pack */
/* --- cacheline 8 boundary (512 bytes) --- */
char cons_outbuf[16]; /* 512 16 */
spinlock_t cons_outbuf_lock; /* 528 4 */
/* XXX 44 bytes hole, try to pack */
/* --- cacheline 9 boundary (576 bytes) --- */
char outbuf[0]; /* 576 0 */
/* size: 576, cachelines: 9, members: 17 */
/* sum members: 476, holes: 2, sum holes: 100 */
};
thanks,
greg k-h
Re: [RESEND PATCH v4 0/8] bpf powerpc: Add BPF_PROBE_MEM support in powerpc JIT compiler
Le 14/10/2021 à 10:15, David Laight a écrit :
From: Hari Bathini
Sent: 12 October 2021 13:31
Patch #1 & #2 are simple cleanup patches. Patch #3 refactors JIT
compiler code with the aim to simplify adding BPF_PROBE_MEM support.
Patch #4 introduces PPC_RAW_BRANCH() macro instead of open coding
branch instruction. Patch #5 & #7 add BPF_PROBE_MEM support for PPC64
& PPC32 JIT compilers respectively. Patch #6 & #8 handle bad userspace
pointers for PPC64 & PPC32 cases respectively.
I thought that BPF was only allowed to do fairly restricted
memory accesses - so WTF does it need a BPF_PROBE_MEM instruction?
Looks like it's been added by commit 2a02759ef5f8 ("bpf: Add support for
BTF pointers to interpreter")
They say in the log:
Pointer to BTF object is a pointer to kernel object or NULL.
The memory access in the interpreter has to be done via
probe_kernel_read
to avoid page faults.
RE: [RESEND PATCH v4 0/8] bpf powerpc: Add BPF_PROBE_MEM support in powerpc JIT compiler
From: Hari Bathini > Sent: 12 October 2021 13:31 > > Patch #1 & #2 are simple cleanup patches. Patch #3 refactors JIT > compiler code with the aim to simplify adding BPF_PROBE_MEM support. > Patch #4 introduces PPC_RAW_BRANCH() macro instead of open coding > branch instruction. Patch #5 & #7 add BPF_PROBE_MEM support for PPC64 > & PPC32 JIT compilers respectively. Patch #6 & #8 handle bad userspace > pointers for PPC64 & PPC32 cases respectively. I thought that BPF was only allowed to do fairly restricted memory accesses - so WTF does it need a BPF_PROBE_MEM instruction? David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
[PATCH] powerpc/pseries/iommu: Add of_node_put() before break
Fix following coccicheck warning:
./arch/powerpc/platforms/pseries/iommu.c:924:1-28: WARNING: Function
for_each_node_with_property should have of_node_put() before break
Early exits from for_each_node_with_property should decrement the
node reference counter.
Signed-off-by: Wan Jiabing
---
arch/powerpc/platforms/pseries/iommu.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/pseries/iommu.c
b/arch/powerpc/platforms/pseries/iommu.c
index 269f61d519c2..c140aa683f66 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -929,8 +929,10 @@ static void find_existing_ddw_windows_named(const char
*name)
}
window = ddw_list_new_entry(pdn, dma64);
- if (!window)
+ if (!window) {
+ of_node_put(pdn);
break;
+ }
spin_lock(_win_list_lock);
list_add(>list, _win_list);
--
2.20.1
Re: [PATCH v1 04/10] asm-generic: Use HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR to define associated stubs
Le 13/10/2021 à 09:00, Kees Cook a écrit : On Mon, Oct 11, 2021 at 05:25:31PM +0200, Christophe Leroy wrote: Use HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR instead of 'dereference_function_descriptor' to know whether arch has function descriptors. Signed-off-by: Christophe Leroy I'd mention the intentionally-empty #if/#else in the commit log, as I, like Helge, mentally tripped over it in the review. :) Reviewed-by: Kees Cook Ok, I did it in v2. I also renamed it HAVE_FUNCTION_DESCRIPTORS because behind the fact that it has some functions to dereference function descriptor, the main fact is that they have and use function descriptors. Christophe
Re: [PATCH] powerpc/64s: Default to 64K pages for 64 bit book3s
On Thu, 14 Oct 2021 at 07:03, LEROY Christophe wrote: > > > > Le 14/10/2021 à 01:31, Joel Stanley a écrit : > > For 64-bit book3s the default should be 64K as that's what modern CPUs > > are designed for. > > > > The following defconfigs already set CONFIG_PPC_64K_PAGES: > > > > cell_defconfig > > pasemi_defconfig > > powernv_defconfig > > ppc64_defconfig > > pseries_defconfig > > skiroot_defconfig > > > > The have the option removed from the defconfig, as it is now the > > default. > > > > The defconfigs that now need to set CONFIG_PPC_4K_PAGES to maintain > > their existing behaviour are: > > > > g5_defconfig > > maple_defconfig > > microwatt_defconfig > > ps3_defconfig > > > > Link: https://github.com/linuxppc/issues/issues/109 > > Signed-off-by: Joel Stanley > > --- > > arch/powerpc/Kconfig | 1 + > > arch/powerpc/configs/cell_defconfig | 1 - > > arch/powerpc/configs/g5_defconfig| 1 + > > arch/powerpc/configs/maple_defconfig | 1 + > > arch/powerpc/configs/microwatt_defconfig | 2 +- > > arch/powerpc/configs/pasemi_defconfig| 1 - > > arch/powerpc/configs/powernv_defconfig | 1 - > > arch/powerpc/configs/ppc64_defconfig | 1 - > > arch/powerpc/configs/ps3_defconfig | 1 + > > arch/powerpc/configs/pseries_defconfig | 1 - > > arch/powerpc/configs/skiroot_defconfig | 1 - > > 11 files changed, 5 insertions(+), 7 deletions(-) > > > > > diff --git a/arch/powerpc/configs/microwatt_defconfig > > b/arch/powerpc/configs/microwatt_defconfig > > index 9465209b8c5b..556ec5eec684 100644 > > --- a/arch/powerpc/configs/microwatt_defconfig > > +++ b/arch/powerpc/configs/microwatt_defconfig > > @@ -1,7 +1,6 @@ > > # CONFIG_SWAP is not set > > # CONFIG_CROSS_MEMORY_ATTACH is not set > > CONFIG_HIGH_RES_TIMERS=y > > -CONFIG_PREEMPT_VOLUNTARY=y > > This seems unrelated. It is, thanks for catching that.
Re: [PATCH] powerpc/64s: Default to 64K pages for 64 bit book3s
Le 14/10/2021 à 01:31, Joel Stanley a écrit : > For 64-bit book3s the default should be 64K as that's what modern CPUs > are designed for. > > The following defconfigs already set CONFIG_PPC_64K_PAGES: > > cell_defconfig > pasemi_defconfig > powernv_defconfig > ppc64_defconfig > pseries_defconfig > skiroot_defconfig > > The have the option removed from the defconfig, as it is now the > default. > > The defconfigs that now need to set CONFIG_PPC_4K_PAGES to maintain > their existing behaviour are: > > g5_defconfig > maple_defconfig > microwatt_defconfig > ps3_defconfig > > Link: https://github.com/linuxppc/issues/issues/109 > Signed-off-by: Joel Stanley > --- > arch/powerpc/Kconfig | 1 + > arch/powerpc/configs/cell_defconfig | 1 - > arch/powerpc/configs/g5_defconfig| 1 + > arch/powerpc/configs/maple_defconfig | 1 + > arch/powerpc/configs/microwatt_defconfig | 2 +- > arch/powerpc/configs/pasemi_defconfig| 1 - > arch/powerpc/configs/powernv_defconfig | 1 - > arch/powerpc/configs/ppc64_defconfig | 1 - > arch/powerpc/configs/ps3_defconfig | 1 + > arch/powerpc/configs/pseries_defconfig | 1 - > arch/powerpc/configs/skiroot_defconfig | 1 - > 11 files changed, 5 insertions(+), 7 deletions(-) > > diff --git a/arch/powerpc/configs/microwatt_defconfig > b/arch/powerpc/configs/microwatt_defconfig > index 9465209b8c5b..556ec5eec684 100644 > --- a/arch/powerpc/configs/microwatt_defconfig > +++ b/arch/powerpc/configs/microwatt_defconfig > @@ -1,7 +1,6 @@ > # CONFIG_SWAP is not set > # CONFIG_CROSS_MEMORY_ATTACH is not set > CONFIG_HIGH_RES_TIMERS=y > -CONFIG_PREEMPT_VOLUNTARY=y This seems unrelated. > CONFIG_TICK_CPU_ACCOUNTING=y > CONFIG_LOG_BUF_SHIFT=16 > CONFIG_PRINTK_SAFE_LOG_BUF_SHIFT=12 > @@ -26,6 +25,7 @@ CONFIG_PPC_MICROWATT=y > # CONFIG_PPC_OF_BOOT_TRAMPOLINE is not set > CONFIG_CPU_FREQ=y > CONFIG_HZ_100=y > +CONFIG_PPC_4K_PAGES=y > # CONFIG_PPC_MEM_KEYS is not set > # CONFIG_SECCOMP is not set > # CONFIG_MQ_IOSCHED_KYBER is not set
Re: [PATCH v2 07/13] asm-generic: Define 'func_desc_t' to commonly describe function descriptors
On Thu, Oct 14, 2021 at 7:49 AM Christophe Leroy wrote: > > We have three architectures using function descriptors, each with its > own name. > > Add a common typedef that can be used in generic code. > > Also add a stub typedef for architecture without function descriptors, > to avoid a forest of #ifdefs. > > It replaces the similar func_desc_t previously defined in > arch/powerpc/kernel/module_64.c > > Reviewed-by: Kees Cook > Signed-off-by: Christophe Leroy Acked-by: Arnd Bergmann
[PATCH v2 08/13] asm-generic: Refactor dereference_[kernel]_function_descriptor()
dereference_function_descriptor() and
dereference_kernel_function_descriptor() are identical on the
three architectures implementing them.
Make them common and put them out-of-line in kernel/extable.c
which is one of the users and has similar type of functions.
Reviewed-by: Kees Cook
Reviewed-by: Arnd Bergmann
Signed-off-by: Christophe Leroy
---
arch/ia64/include/asm/sections.h| 19 ---
arch/parisc/include/asm/sections.h | 9 -
arch/parisc/kernel/process.c| 21 -
arch/powerpc/include/asm/sections.h | 23 ---
include/asm-generic/sections.h | 2 ++
kernel/extable.c| 23 ++-
6 files changed, 24 insertions(+), 73 deletions(-)
diff --git a/arch/ia64/include/asm/sections.h b/arch/ia64/include/asm/sections.h
index 1aaed8882294..96c9bb500c34 100644
--- a/arch/ia64/include/asm/sections.h
+++ b/arch/ia64/include/asm/sections.h
@@ -31,23 +31,4 @@ extern char __start_gate_brl_fsys_bubble_down_patchlist[],
__end_gate_brl_fsys_b
extern char __start_unwind[], __end_unwind[];
extern char __start_ivt_text[], __end_ivt_text[];
-#undef dereference_function_descriptor
-static inline void *dereference_function_descriptor(void *ptr)
-{
- struct fdesc *desc = ptr;
- void *p;
-
- if (!get_kernel_nofault(p, (void *)>addr))
- ptr = p;
- return ptr;
-}
-
-#undef dereference_kernel_function_descriptor
-static inline void *dereference_kernel_function_descriptor(void *ptr)
-{
- if (ptr < (void *)__start_opd || ptr >= (void *)__end_opd)
- return ptr;
- return dereference_function_descriptor(ptr);
-}
-
#endif /* _ASM_IA64_SECTIONS_H */
diff --git a/arch/parisc/include/asm/sections.h
b/arch/parisc/include/asm/sections.h
index 37b34b357cb5..6b1fe22baaf5 100644
--- a/arch/parisc/include/asm/sections.h
+++ b/arch/parisc/include/asm/sections.h
@@ -13,13 +13,4 @@ typedef Elf64_Fdesc func_desc_t;
extern char __alt_instructions[], __alt_instructions_end[];
-#ifdef CONFIG_64BIT
-
-#undef dereference_function_descriptor
-void *dereference_function_descriptor(void *);
-
-#undef dereference_kernel_function_descriptor
-void *dereference_kernel_function_descriptor(void *);
-#endif
-
#endif
diff --git a/arch/parisc/kernel/process.c b/arch/parisc/kernel/process.c
index 38ec4ae81239..7382576b52a8 100644
--- a/arch/parisc/kernel/process.c
+++ b/arch/parisc/kernel/process.c
@@ -266,27 +266,6 @@ get_wchan(struct task_struct *p)
return 0;
}
-#ifdef CONFIG_64BIT
-void *dereference_function_descriptor(void *ptr)
-{
- Elf64_Fdesc *desc = ptr;
- void *p;
-
- if (!get_kernel_nofault(p, (void *)>addr))
- ptr = p;
- return ptr;
-}
-
-void *dereference_kernel_function_descriptor(void *ptr)
-{
- if (ptr < (void *)__start_opd ||
- ptr >= (void *)__end_opd)
- return ptr;
-
- return dereference_function_descriptor(ptr);
-}
-#endif
-
static inline unsigned long brk_rnd(void)
{
return (get_random_int() & BRK_RND_MASK) << PAGE_SHIFT;
diff --git a/arch/powerpc/include/asm/sections.h
b/arch/powerpc/include/asm/sections.h
index 1322d7b2f1a3..fbfe1957edbe 100644
--- a/arch/powerpc/include/asm/sections.h
+++ b/arch/powerpc/include/asm/sections.h
@@ -72,29 +72,6 @@ static inline int overlaps_kernel_text(unsigned long start,
unsigned long end)
(unsigned long)_stext < end;
}
-#ifdef PPC64_ELF_ABI_v1
-
-#undef dereference_function_descriptor
-static inline void *dereference_function_descriptor(void *ptr)
-{
- struct ppc64_opd_entry *desc = ptr;
- void *p;
-
- if (!get_kernel_nofault(p, (void *)>addr))
- ptr = p;
- return ptr;
-}
-
-#undef dereference_kernel_function_descriptor
-static inline void *dereference_kernel_function_descriptor(void *ptr)
-{
- if (ptr < (void *)__start_opd || ptr >= (void *)__end_opd)
- return ptr;
-
- return dereference_function_descriptor(ptr);
-}
-#endif /* PPC64_ELF_ABI_v1 */
-
#endif
#endif /* __KERNEL__ */
diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
index cbec7d5f1678..76163883c6ff 100644
--- a/include/asm-generic/sections.h
+++ b/include/asm-generic/sections.h
@@ -60,6 +60,8 @@ extern __visible const void __nosave_begin, __nosave_end;
/* Function descriptor handling (if any). Override in asm/sections.h */
#ifdef HAVE_FUNCTION_DESCRIPTORS
+void *dereference_function_descriptor(void *ptr);
+void *dereference_kernel_function_descriptor(void *ptr);
#else
#define dereference_function_descriptor(p) ((void *)(p))
#define dereference_kernel_function_descriptor(p) ((void *)(p))
diff --git a/kernel/extable.c b/kernel/extable.c
index b0ea5eb0c3b4..013ccffade11 100644
--- a/kernel/extable.c
+++ b/kernel/extable.c
@@ -3,6 +3,7 @@
Copyright (C) 2001 Rusty Russell, 2002 Rusty Russell IBM.
*/
+#include
#include
[PATCH v2 06/13] asm-generic: Use HAVE_FUNCTION_DESCRIPTORS to define associated stubs
Replace HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR by
HAVE_FUNCTION_DESCRIPTORS and use it instead of
'dereference_function_descriptor' macro to know
whether an arch has function descriptors.
To limit churn in one of the following patches, use
an #ifdef/#else construct with empty first part
instead of an #ifndef in asm-generic/sections.h
Reviewed-by: Kees Cook
Signed-off-by: Christophe Leroy
---
arch/ia64/include/asm/sections.h| 5 +++--
arch/parisc/include/asm/sections.h | 6 --
arch/powerpc/include/asm/sections.h | 6 --
include/asm-generic/sections.h | 3 ++-
include/linux/kallsyms.h| 2 +-
5 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/arch/ia64/include/asm/sections.h b/arch/ia64/include/asm/sections.h
index 35f24e52149a..6e55e545bf02 100644
--- a/arch/ia64/include/asm/sections.h
+++ b/arch/ia64/include/asm/sections.h
@@ -9,6 +9,9 @@
#include
#include
+
+#define HAVE_FUNCTION_DESCRIPTORS 1
+
#include
extern char __phys_per_cpu_start[];
@@ -27,8 +30,6 @@ extern char __start_gate_brl_fsys_bubble_down_patchlist[],
__end_gate_brl_fsys_b
extern char __start_unwind[], __end_unwind[];
extern char __start_ivt_text[], __end_ivt_text[];
-#define HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR 1
-
#undef dereference_function_descriptor
static inline void *dereference_function_descriptor(void *ptr)
{
diff --git a/arch/parisc/include/asm/sections.h
b/arch/parisc/include/asm/sections.h
index bb52aea0cb21..85149a89ff3e 100644
--- a/arch/parisc/include/asm/sections.h
+++ b/arch/parisc/include/asm/sections.h
@@ -2,6 +2,10 @@
#ifndef _PARISC_SECTIONS_H
#define _PARISC_SECTIONS_H
+#ifdef CONFIG_64BIT
+#define HAVE_FUNCTION_DESCRIPTORS 1
+#endif
+
/* nothing to see, move along */
#include
@@ -9,8 +13,6 @@ extern char __alt_instructions[], __alt_instructions_end[];
#ifdef CONFIG_64BIT
-#define HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR 1
-
#undef dereference_function_descriptor
void *dereference_function_descriptor(void *);
diff --git a/arch/powerpc/include/asm/sections.h
b/arch/powerpc/include/asm/sections.h
index 32e7035863ac..bba97b8c38cf 100644
--- a/arch/powerpc/include/asm/sections.h
+++ b/arch/powerpc/include/asm/sections.h
@@ -8,6 +8,10 @@
#define arch_is_kernel_initmem_freed arch_is_kernel_initmem_freed
+#ifdef PPC64_ELF_ABI_v1
+#define HAVE_FUNCTION_DESCRIPTORS 1
+#endif
+
#include
extern bool init_mem_is_free;
@@ -69,8 +73,6 @@ static inline int overlaps_kernel_text(unsigned long start,
unsigned long end)
#ifdef PPC64_ELF_ABI_v1
-#define HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR 1
-
#undef dereference_function_descriptor
static inline void *dereference_function_descriptor(void *ptr)
{
diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
index d16302d3eb59..b677e926e6b3 100644
--- a/include/asm-generic/sections.h
+++ b/include/asm-generic/sections.h
@@ -59,7 +59,8 @@ extern char __noinstr_text_start[], __noinstr_text_end[];
extern __visible const void __nosave_begin, __nosave_end;
/* Function descriptor handling (if any). Override in asm/sections.h */
-#ifndef dereference_function_descriptor
+#ifdef HAVE_FUNCTION_DESCRIPTORS
+#else
#define dereference_function_descriptor(p) ((void *)(p))
#define dereference_kernel_function_descriptor(p) ((void *)(p))
#endif
diff --git a/include/linux/kallsyms.h b/include/linux/kallsyms.h
index a1d6fc82d7f0..9f277baeb559 100644
--- a/include/linux/kallsyms.h
+++ b/include/linux/kallsyms.h
@@ -57,7 +57,7 @@ static inline int is_ksym_addr(unsigned long addr)
static inline void *dereference_symbol_descriptor(void *ptr)
{
-#ifdef HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR
+#ifdef HAVE_FUNCTION_DESCRIPTORS
struct module *mod;
ptr = dereference_kernel_function_descriptor(ptr);
--
2.31.1
[PATCH v2 12/13] lkdtm: Fix execute_[user]_location()
execute_location() and execute_user_location() intent
to copy do_nothing() text and execute it at a new location.
However, at the time being it doesn't copy do_nothing() function
but do_nothing() function descriptor which still points to the
original text. So at the end it still executes do_nothing() at
its original location allthough using a copied function descriptor.
So, fix that by really copying do_nothing() text and build a new
function descriptor by copying do_nothing() function descriptor and
updating the target address with the new location.
Also fix the displayed addresses by dereferencing do_nothing()
function descriptor.
Signed-off-by: Christophe Leroy
---
drivers/misc/lkdtm/perms.c | 25 +
include/asm-generic/sections.h | 5 +
2 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/drivers/misc/lkdtm/perms.c b/drivers/misc/lkdtm/perms.c
index 5266dc28df6e..96b3ebfcb8ed 100644
--- a/drivers/misc/lkdtm/perms.c
+++ b/drivers/misc/lkdtm/perms.c
@@ -44,19 +44,32 @@ static noinline void do_overwritten(void)
return;
}
+static void *setup_function_descriptor(func_desc_t *fdesc, void *dst)
+{
+ memcpy(fdesc, do_nothing, sizeof(*fdesc));
+ fdesc->addr = (unsigned long)dst;
+ barrier();
+
+ return fdesc;
+}
+
static noinline void execute_location(void *dst, bool write)
{
void (*func)(void) = dst;
+ func_desc_t fdesc;
+ void *do_nothing_text = dereference_function_descriptor(do_nothing);
- pr_info("attempting ok execution at %px\n", do_nothing);
+ pr_info("attempting ok execution at %px\n", do_nothing_text);
do_nothing();
if (write == CODE_WRITE) {
- memcpy(dst, do_nothing, EXEC_SIZE);
+ memcpy(dst, do_nothing_text, EXEC_SIZE);
flush_icache_range((unsigned long)dst,
(unsigned long)dst + EXEC_SIZE);
}
pr_info("attempting bad execution at %px\n", func);
+ if (have_function_descriptors())
+ func = setup_function_descriptor(, dst);
func();
pr_err("FAIL: func returned\n");
}
@@ -67,15 +80,19 @@ static void execute_user_location(void *dst)
/* Intentionally crossing kernel/user memory boundary. */
void (*func)(void) = dst;
+ func_desc_t fdesc;
+ void *do_nothing_text = dereference_function_descriptor(do_nothing);
- pr_info("attempting ok execution at %px\n", do_nothing);
+ pr_info("attempting ok execution at %px\n", do_nothing_text);
do_nothing();
- copied = access_process_vm(current, (unsigned long)dst, do_nothing,
+ copied = access_process_vm(current, (unsigned long)dst, do_nothing_text,
EXEC_SIZE, FOLL_WRITE);
if (copied < EXEC_SIZE)
return;
pr_info("attempting bad execution at %px\n", func);
+ if (have_function_descriptors())
+ func = setup_function_descriptor(, dst);
func();
pr_err("FAIL: func returned\n");
}
diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
index 76163883c6ff..d225318538bd 100644
--- a/include/asm-generic/sections.h
+++ b/include/asm-generic/sections.h
@@ -70,6 +70,11 @@ typedef struct {
} func_desc_t;
#endif
+static inline bool have_function_descriptors(void)
+{
+ return __is_defined(HAVE_FUNCTION_DESCRIPTORS);
+}
+
/* random extra sections (if any). Override
* in asm/sections.h */
#ifndef arch_is_kernel_text
--
2.31.1
[PATCH v2 07/13] asm-generic: Define 'func_desc_t' to commonly describe function descriptors
We have three architectures using function descriptors, each with its
own name.
Add a common typedef that can be used in generic code.
Also add a stub typedef for architecture without function descriptors,
to avoid a forest of #ifdefs.
It replaces the similar func_desc_t previously defined in
arch/powerpc/kernel/module_64.c
Reviewed-by: Kees Cook
Signed-off-by: Christophe Leroy
---
arch/ia64/include/asm/sections.h| 1 +
arch/parisc/include/asm/sections.h | 2 ++
arch/powerpc/include/asm/sections.h | 1 +
arch/powerpc/kernel/module_64.c | 8
include/asm-generic/sections.h | 3 +++
5 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/arch/ia64/include/asm/sections.h b/arch/ia64/include/asm/sections.h
index 6e55e545bf02..1aaed8882294 100644
--- a/arch/ia64/include/asm/sections.h
+++ b/arch/ia64/include/asm/sections.h
@@ -11,6 +11,7 @@
#include
#define HAVE_FUNCTION_DESCRIPTORS 1
+typedef struct fdesc func_desc_t;
#include
diff --git a/arch/parisc/include/asm/sections.h
b/arch/parisc/include/asm/sections.h
index 85149a89ff3e..37b34b357cb5 100644
--- a/arch/parisc/include/asm/sections.h
+++ b/arch/parisc/include/asm/sections.h
@@ -3,7 +3,9 @@
#define _PARISC_SECTIONS_H
#ifdef CONFIG_64BIT
+#include
#define HAVE_FUNCTION_DESCRIPTORS 1
+typedef Elf64_Fdesc func_desc_t;
#endif
/* nothing to see, move along */
diff --git a/arch/powerpc/include/asm/sections.h
b/arch/powerpc/include/asm/sections.h
index bba97b8c38cf..1322d7b2f1a3 100644
--- a/arch/powerpc/include/asm/sections.h
+++ b/arch/powerpc/include/asm/sections.h
@@ -10,6 +10,7 @@
#ifdef PPC64_ELF_ABI_v1
#define HAVE_FUNCTION_DESCRIPTORS 1
+typedef struct ppc64_opd_entry func_desc_t;
#endif
#include
diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
index dc99a3f6cee2..affda7698242 100644
--- a/arch/powerpc/kernel/module_64.c
+++ b/arch/powerpc/kernel/module_64.c
@@ -32,11 +32,6 @@
#ifdef PPC64_ELF_ABI_v2
-/* An address is simply the address of the function. */
-typedef struct {
- unsigned long addr;
-} func_desc_t;
-
static func_desc_t func_desc(unsigned long addr)
{
return (func_desc_t){addr};
@@ -57,9 +52,6 @@ static unsigned int local_entry_offset(const Elf64_Sym *sym)
}
#else
-/* An address is address of the OPD entry, which contains address of fn. */
-typedef struct ppc64_opd_entry func_desc_t;
-
static func_desc_t func_desc(unsigned long addr)
{
return *(func_desc_t *)addr;
diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
index b677e926e6b3..cbec7d5f1678 100644
--- a/include/asm-generic/sections.h
+++ b/include/asm-generic/sections.h
@@ -63,6 +63,9 @@ extern __visible const void __nosave_begin, __nosave_end;
#else
#define dereference_function_descriptor(p) ((void *)(p))
#define dereference_kernel_function_descriptor(p) ((void *)(p))
+typedef struct {
+ unsigned long addr;
+} func_desc_t;
#endif
/* random extra sections (if any). Override
--
2.31.1
[PATCH v2 13/13] lkdtm: Add a test for function descriptors protection
Add WRITE_OPD to check that you can't modify function
descriptors.
Gives the following result when function descriptors are
not protected:
lkdtm: Performing direct entry WRITE_OPD
lkdtm: attempting bad 16 bytes write at c269b358
lkdtm: FAIL: survived bad write
lkdtm: do_nothing was hijacked!
Looks like a standard compiler barrier(); is not enough to force
GCC to use the modified function descriptor. Add to add a fake empty
inline assembly to force GCC to reload the function descriptor.
Signed-off-by: Christophe Leroy
---
drivers/misc/lkdtm/core.c | 1 +
drivers/misc/lkdtm/lkdtm.h | 1 +
drivers/misc/lkdtm/perms.c | 22 ++
3 files changed, 24 insertions(+)
diff --git a/drivers/misc/lkdtm/core.c b/drivers/misc/lkdtm/core.c
index fe6fd34b8caf..de092aa03b5d 100644
--- a/drivers/misc/lkdtm/core.c
+++ b/drivers/misc/lkdtm/core.c
@@ -148,6 +148,7 @@ static const struct crashtype crashtypes[] = {
CRASHTYPE(WRITE_RO),
CRASHTYPE(WRITE_RO_AFTER_INIT),
CRASHTYPE(WRITE_KERN),
+ CRASHTYPE(WRITE_OPD),
CRASHTYPE(REFCOUNT_INC_OVERFLOW),
CRASHTYPE(REFCOUNT_ADD_OVERFLOW),
CRASHTYPE(REFCOUNT_INC_NOT_ZERO_OVERFLOW),
diff --git a/drivers/misc/lkdtm/lkdtm.h b/drivers/misc/lkdtm/lkdtm.h
index c212a253edde..188bd0fd6575 100644
--- a/drivers/misc/lkdtm/lkdtm.h
+++ b/drivers/misc/lkdtm/lkdtm.h
@@ -105,6 +105,7 @@ void __init lkdtm_perms_init(void);
void lkdtm_WRITE_RO(void);
void lkdtm_WRITE_RO_AFTER_INIT(void);
void lkdtm_WRITE_KERN(void);
+void lkdtm_WRITE_OPD(void);
void lkdtm_EXEC_DATA(void);
void lkdtm_EXEC_STACK(void);
void lkdtm_EXEC_KMALLOC(void);
diff --git a/drivers/misc/lkdtm/perms.c b/drivers/misc/lkdtm/perms.c
index 96b3ebfcb8ed..3870bc82d40d 100644
--- a/drivers/misc/lkdtm/perms.c
+++ b/drivers/misc/lkdtm/perms.c
@@ -44,6 +44,11 @@ static noinline void do_overwritten(void)
return;
}
+static noinline void do_almost_nothing(void)
+{
+ pr_info("do_nothing was hijacked!\n");
+}
+
static void *setup_function_descriptor(func_desc_t *fdesc, void *dst)
{
memcpy(fdesc, do_nothing, sizeof(*fdesc));
@@ -143,6 +148,23 @@ void lkdtm_WRITE_KERN(void)
do_overwritten();
}
+void lkdtm_WRITE_OPD(void)
+{
+ size_t size = sizeof(func_desc_t);
+ void (*func)(void) = do_nothing;
+
+ if (!have_function_descriptors()) {
+ pr_info("Platform doesn't have function descriptors.\n");
+ return;
+ }
+ pr_info("attempting bad %zu bytes write at %px\n", size, do_nothing);
+ memcpy(do_nothing, do_almost_nothing, size);
+ pr_err("FAIL: survived bad write\n");
+
+ asm("" : "=m"(func));
+ func();
+}
+
void lkdtm_EXEC_DATA(void)
{
execute_location(data_area, CODE_WRITE);
--
2.31.1
[PATCH v2 02/13] powerpc: Rename 'funcaddr' to 'addr' in 'struct ppc64_opd_entry'
There are three architectures with function descriptors, try to
have common names for the address they contain in order to
refactor some functions into generic functions later.
powerpc has 'funcaddr'
ia64 has 'ip'
parisc has 'addr'
Vote for 'addr' and update 'struct ppc64_opd_entry' accordingly.
Reviewed-by: Kees Cook
Signed-off-by: Christophe Leroy
---
arch/powerpc/include/asm/elf.h | 2 +-
arch/powerpc/include/asm/sections.h | 2 +-
arch/powerpc/kernel/module_64.c | 6 +++---
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/include/asm/elf.h b/arch/powerpc/include/asm/elf.h
index a4406714c060..bb0f278f9ed4 100644
--- a/arch/powerpc/include/asm/elf.h
+++ b/arch/powerpc/include/asm/elf.h
@@ -178,7 +178,7 @@ void relocate(unsigned long final_address);
/* There's actually a third entry here, but it's unused */
struct ppc64_opd_entry {
- unsigned long funcaddr;
+ unsigned long addr;
unsigned long r2;
};
diff --git a/arch/powerpc/include/asm/sections.h
b/arch/powerpc/include/asm/sections.h
index 6e4af4492a14..32e7035863ac 100644
--- a/arch/powerpc/include/asm/sections.h
+++ b/arch/powerpc/include/asm/sections.h
@@ -77,7 +77,7 @@ static inline void *dereference_function_descriptor(void *ptr)
struct ppc64_opd_entry *desc = ptr;
void *p;
- if (!get_kernel_nofault(p, (void *)>funcaddr))
+ if (!get_kernel_nofault(p, (void *)>addr))
ptr = p;
return ptr;
}
diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
index 6baa676e7cb6..82908c9be627 100644
--- a/arch/powerpc/kernel/module_64.c
+++ b/arch/powerpc/kernel/module_64.c
@@ -72,11 +72,11 @@ static func_desc_t func_desc(unsigned long addr)
}
static unsigned long func_addr(unsigned long addr)
{
- return func_desc(addr).funcaddr;
+ return func_desc(addr).addr;
}
static unsigned long stub_func_addr(func_desc_t func)
{
- return func.funcaddr;
+ return func.addr;
}
static unsigned int local_entry_offset(const Elf64_Sym *sym)
{
@@ -187,7 +187,7 @@ static int relacmp(const void *_x, const void *_y)
static unsigned long get_stubs_size(const Elf64_Ehdr *hdr,
const Elf64_Shdr *sechdrs)
{
- /* One extra reloc so it's always 0-funcaddr terminated */
+ /* One extra reloc so it's always 0-addr terminated */
unsigned long relocs = 1;
unsigned i;
--
2.31.1
[PATCH v2 00/13] Fix LKDTM for PPC64/IA64/PARISC
PPC64/IA64/PARISC have function descriptors. LKDTM doesn't work on those three architectures because LKDTM messes up function descriptors with functions. This series does some cleanup in the three architectures and refactors function descriptors so that it can then easily use it in a generic way in LKDTM. Patch 8 is not absolutely necessary but it is a good trivial cleanup. Changes in v2: - Addressed received comments - Moved dereference_[kernel]_function_descriptor() out of line - Added patches to remove func_descr_t and func_desc_t in powerpc - Using func_desc_t instead of funct_descr_t - Renamed HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR to HAVE_FUNCTION_DESCRIPTORS - Added a new lkdtm test to check protection of function descriptors Christophe Leroy (13): powerpc: Move 'struct ppc64_opd_entry' back into asm/elf.h powerpc: Rename 'funcaddr' to 'addr' in 'struct ppc64_opd_entry' powerpc: Remove func_descr_t powerpc: Prepare func_desc_t for refactorisation ia64: Rename 'ip' to 'addr' in 'struct fdesc' asm-generic: Use HAVE_FUNCTION_DESCRIPTORS to define associated stubs asm-generic: Define 'func_desc_t' to commonly describe function descriptors asm-generic: Refactor dereference_[kernel]_function_descriptor() lkdtm: Force do_nothing() out of line lkdtm: Really write into kernel text in WRITE_KERN lkdtm: Fix lkdtm_EXEC_RODATA() lkdtm: Fix execute_[user]_location() lkdtm: Add a test for function descriptors protection arch/ia64/include/asm/elf.h | 2 +- arch/ia64/include/asm/sections.h | 25 ++--- arch/ia64/kernel/module.c| 6 +-- arch/parisc/include/asm/sections.h | 17 +++--- arch/parisc/kernel/process.c | 21 arch/powerpc/include/asm/code-patching.h | 2 +- arch/powerpc/include/asm/elf.h | 6 +++ arch/powerpc/include/asm/sections.h | 30 ++- arch/powerpc/include/asm/types.h | 6 --- arch/powerpc/include/uapi/asm/elf.h | 8 --- arch/powerpc/kernel/module_64.c | 38 + arch/powerpc/kernel/signal_64.c | 8 +-- drivers/misc/lkdtm/core.c| 1 + drivers/misc/lkdtm/lkdtm.h | 1 + drivers/misc/lkdtm/perms.c | 68 include/asm-generic/sections.h | 13 - include/linux/kallsyms.h | 2 +- kernel/extable.c | 23 +++- 18 files changed, 138 insertions(+), 139 deletions(-) -- 2.31.1
[PATCH v2 03/13] powerpc: Remove func_descr_t
'func_descr_t' is redundant with 'struct ppc64_opd_entry'
Remove it.
Signed-off-by: Christophe Leroy
---
arch/powerpc/include/asm/code-patching.h | 2 +-
arch/powerpc/include/asm/types.h | 6 --
arch/powerpc/kernel/signal_64.c | 8
3 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/include/asm/code-patching.h
b/arch/powerpc/include/asm/code-patching.h
index 4ba834599c4d..f3445188d319 100644
--- a/arch/powerpc/include/asm/code-patching.h
+++ b/arch/powerpc/include/asm/code-patching.h
@@ -110,7 +110,7 @@ static inline unsigned long ppc_function_entry(void *func)
* function's descriptor. The first entry in the descriptor is the
* address of the function text.
*/
- return ((func_descr_t *)func)->entry;
+ return ((struct ppc64_opd_entry *)func)->addr;
#else
return (unsigned long)func;
#endif
diff --git a/arch/powerpc/include/asm/types.h b/arch/powerpc/include/asm/types.h
index f1630c553efe..97da77bc48c9 100644
--- a/arch/powerpc/include/asm/types.h
+++ b/arch/powerpc/include/asm/types.h
@@ -23,12 +23,6 @@
typedef __vector128 vector128;
-typedef struct {
- unsigned long entry;
- unsigned long toc;
- unsigned long env;
-} func_descr_t;
-
#endif /* __ASSEMBLY__ */
#endif /* _ASM_POWERPC_TYPES_H */
diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c
index 1831bba0582e..63ddbe7b108c 100644
--- a/arch/powerpc/kernel/signal_64.c
+++ b/arch/powerpc/kernel/signal_64.c
@@ -933,11 +933,11 @@ int handle_rt_signal64(struct ksignal *ksig, sigset_t
*set,
* descriptor is the entry address of signal and the second
* entry is the TOC value we need to use.
*/
- func_descr_t __user *funct_desc_ptr =
- (func_descr_t __user *) ksig->ka.sa.sa_handler;
+ struct ppc64_opd_entry __user *funct_desc_ptr =
+ (struct ppc64_opd_entry __user *)ksig->ka.sa.sa_handler;
- err |= get_user(regs->ctr, _desc_ptr->entry);
- err |= get_user(regs->gpr[2], _desc_ptr->toc);
+ err |= get_user(regs->ctr, _desc_ptr->addr);
+ err |= get_user(regs->gpr[2], _desc_ptr->r2);
}
/* enter the signal handler in native-endian mode */
--
2.31.1
[PATCH v2 05/13] ia64: Rename 'ip' to 'addr' in 'struct fdesc'
There are three architectures with function descriptors, try to
have common names for the address they contain in order to
refactor some functions into generic functions later.
powerpc has 'funcaddr'
ia64 has 'ip'
parisc has 'addr'
Vote for 'addr' and update 'struct fdesc' accordingly.
Reviewed-by: Kees Cook
Signed-off-by: Christophe Leroy
---
arch/ia64/include/asm/elf.h | 2 +-
arch/ia64/include/asm/sections.h | 2 +-
arch/ia64/kernel/module.c| 6 +++---
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/ia64/include/asm/elf.h b/arch/ia64/include/asm/elf.h
index 6629301a2620..2ef5f9966ad1 100644
--- a/arch/ia64/include/asm/elf.h
+++ b/arch/ia64/include/asm/elf.h
@@ -226,7 +226,7 @@ struct got_entry {
* Layout of the Function Descriptor
*/
struct fdesc {
- uint64_t ip;
+ uint64_t addr;
uint64_t gp;
};
diff --git a/arch/ia64/include/asm/sections.h b/arch/ia64/include/asm/sections.h
index 3a033d2008b3..35f24e52149a 100644
--- a/arch/ia64/include/asm/sections.h
+++ b/arch/ia64/include/asm/sections.h
@@ -35,7 +35,7 @@ static inline void *dereference_function_descriptor(void *ptr)
struct fdesc *desc = ptr;
void *p;
- if (!get_kernel_nofault(p, (void *)>ip))
+ if (!get_kernel_nofault(p, (void *)>addr))
ptr = p;
return ptr;
}
diff --git a/arch/ia64/kernel/module.c b/arch/ia64/kernel/module.c
index 2cba53c1da82..4f6400cbf79e 100644
--- a/arch/ia64/kernel/module.c
+++ b/arch/ia64/kernel/module.c
@@ -602,15 +602,15 @@ get_fdesc (struct module *mod, uint64_t value, int *okp)
return value;
/* Look for existing function descriptor. */
- while (fdesc->ip) {
- if (fdesc->ip == value)
+ while (fdesc->addr) {
+ if (fdesc->addr == value)
return (uint64_t)fdesc;
if ((uint64_t) ++fdesc >= mod->arch.opd->sh_addr +
mod->arch.opd->sh_size)
BUG();
}
/* Create new one */
- fdesc->ip = value;
+ fdesc->addr = value;
fdesc->gp = mod->arch.gp;
return (uint64_t) fdesc;
}
--
2.31.1
[PATCH v2 04/13] powerpc: Prepare func_desc_t for refactorisation
In preparation of making func_desc_t generic, change it to
a struct containing 'addr' element.
In addition this allows using single helpers common to ELFv1 and ELFv2.
Signed-off-by: Christophe Leroy
---
arch/powerpc/kernel/module_64.c | 34 +++--
1 file changed, 15 insertions(+), 19 deletions(-)
diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
index 82908c9be627..dc99a3f6cee2 100644
--- a/arch/powerpc/kernel/module_64.c
+++ b/arch/powerpc/kernel/module_64.c
@@ -33,19 +33,13 @@
#ifdef PPC64_ELF_ABI_v2
/* An address is simply the address of the function. */
-typedef unsigned long func_desc_t;
+typedef struct {
+ unsigned long addr;
+} func_desc_t;
static func_desc_t func_desc(unsigned long addr)
{
- return addr;
-}
-static unsigned long func_addr(unsigned long addr)
-{
- return addr;
-}
-static unsigned long stub_func_addr(func_desc_t func)
-{
- return func;
+ return (func_desc_t){addr};
}
/* PowerPC64 specific values for the Elf64_Sym st_other field. */
@@ -68,15 +62,7 @@ typedef struct ppc64_opd_entry func_desc_t;
static func_desc_t func_desc(unsigned long addr)
{
- return *(struct ppc64_opd_entry *)addr;
-}
-static unsigned long func_addr(unsigned long addr)
-{
- return func_desc(addr).addr;
-}
-static unsigned long stub_func_addr(func_desc_t func)
-{
- return func.addr;
+ return *(func_desc_t *)addr;
}
static unsigned int local_entry_offset(const Elf64_Sym *sym)
{
@@ -93,6 +79,16 @@ void *dereference_module_function_descriptor(struct module
*mod, void *ptr)
}
#endif
+static unsigned long func_addr(unsigned long addr)
+{
+ return func_desc(addr).addr;
+}
+
+static unsigned long stub_func_addr(func_desc_t func)
+{
+ return func.addr;
+}
+
#define STUB_MAGIC 0x73747562 /* stub */
/* Like PPC32, we need little trampolines to do > 24-bit jumps (into
--
2.31.1
[PATCH v2 01/13] powerpc: Move 'struct ppc64_opd_entry' back into asm/elf.h
'struct ppc64_opd_entry' doesn't belong to uapi/asm/elf.h
It was initially in module_64.c and commit 2d291e902791 ("Fix compile
failure with non modular builds") moved it into asm/elf.h
But it was by mistake added outside of __KERNEL__ section,
therefore commit c3617f72036c ("UAPI: (Scripted) Disintegrate
arch/powerpc/include/asm") moved it to uapi/asm/elf.h
Move it back into asm/elf.h, this brings it back in line with
IA64 and PARISC architectures.
Fixes: 2d291e902791 ("Fix compile failure with non modular builds")
Reviewed-by: Kees Cook
Signed-off-by: Christophe Leroy
---
arch/powerpc/include/asm/elf.h | 6 ++
arch/powerpc/include/uapi/asm/elf.h | 8
2 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/include/asm/elf.h b/arch/powerpc/include/asm/elf.h
index b8425e3cfd81..a4406714c060 100644
--- a/arch/powerpc/include/asm/elf.h
+++ b/arch/powerpc/include/asm/elf.h
@@ -176,4 +176,10 @@ do {
\
/* Relocate the kernel image to @final_address */
void relocate(unsigned long final_address);
+/* There's actually a third entry here, but it's unused */
+struct ppc64_opd_entry {
+ unsigned long funcaddr;
+ unsigned long r2;
+};
+
#endif /* _ASM_POWERPC_ELF_H */
diff --git a/arch/powerpc/include/uapi/asm/elf.h
b/arch/powerpc/include/uapi/asm/elf.h
index 860c59291bfc..308857123a08 100644
--- a/arch/powerpc/include/uapi/asm/elf.h
+++ b/arch/powerpc/include/uapi/asm/elf.h
@@ -289,12 +289,4 @@ typedef elf_fpreg_t elf_vsrreghalf_t32[ELF_NVSRHALFREG];
/* Keep this the last entry. */
#define R_PPC64_NUM253
-/* There's actually a third entry here, but it's unused */
-struct ppc64_opd_entry
-{
- unsigned long funcaddr;
- unsigned long r2;
-};
-
-
#endif /* _UAPI_ASM_POWERPC_ELF_H */
--
2.31.1
[PATCH v2 10/13] lkdtm: Really write into kernel text in WRITE_KERN
WRITE_KERN is supposed to overwrite some kernel text, namely
do_overwritten() function.
But at the time being it overwrites do_overwritten() function
descriptor, not function text.
Fix it by dereferencing the function descriptor to obtain
function text pointer.
And make do_overwritten() noinline so that it is really
do_overwritten() which is called by lkdtm_WRITE_KERN().
Acked-by: Kees Cook
Signed-off-by: Christophe Leroy
---
drivers/misc/lkdtm/perms.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/misc/lkdtm/perms.c b/drivers/misc/lkdtm/perms.c
index 60b3b2fe929d..035fcca441f0 100644
--- a/drivers/misc/lkdtm/perms.c
+++ b/drivers/misc/lkdtm/perms.c
@@ -10,6 +10,7 @@
#include
#include
#include
+#include
/* Whether or not to fill the target memory area with do_nothing(). */
#define CODE_WRITE true
@@ -37,7 +38,7 @@ static noinline void do_nothing(void)
}
/* Must immediately follow do_nothing for size calculuations to work out. */
-static void do_overwritten(void)
+static noinline void do_overwritten(void)
{
pr_info("do_overwritten wasn't overwritten!\n");
return;
@@ -113,8 +114,9 @@ void lkdtm_WRITE_KERN(void)
size_t size;
volatile unsigned char *ptr;
- size = (unsigned long)do_overwritten - (unsigned long)do_nothing;
- ptr = (unsigned char *)do_overwritten;
+ size = (unsigned long)dereference_function_descriptor(do_overwritten) -
+ (unsigned long)dereference_function_descriptor(do_nothing);
+ ptr = dereference_function_descriptor(do_overwritten);
pr_info("attempting bad %zu byte write at %px\n", size, ptr);
memcpy((void *)ptr, (unsigned char *)do_nothing, size);
--
2.31.1
[PATCH v2 11/13] lkdtm: Fix lkdtm_EXEC_RODATA()
Behind its location, lkdtm_EXEC_RODATA() executes
lkdtm_rodata_do_nothing() which is a real function,
not a copy of do_nothing().
So executes it directly instead of using execute_location().
This is necessary because following patch will fix execute_location()
to use a copy of the function descriptor of do_nothing() and
function descriptor of lkdtm_rodata_do_nothing() might be different.
And fix displayed addresses by dereferencing the function descriptors.
Signed-off-by: Christophe Leroy
---
drivers/misc/lkdtm/perms.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/misc/lkdtm/perms.c b/drivers/misc/lkdtm/perms.c
index 035fcca441f0..5266dc28df6e 100644
--- a/drivers/misc/lkdtm/perms.c
+++ b/drivers/misc/lkdtm/perms.c
@@ -153,7 +153,14 @@ void lkdtm_EXEC_VMALLOC(void)
void lkdtm_EXEC_RODATA(void)
{
- execute_location(lkdtm_rodata_do_nothing, CODE_AS_IS);
+ pr_info("attempting ok execution at %px\n",
+ dereference_function_descriptor(do_nothing));
+ do_nothing();
+
+ pr_info("attempting bad execution at %px\n",
+ dereference_function_descriptor(lkdtm_rodata_do_nothing));
+ lkdtm_rodata_do_nothing();
+ pr_err("FAIL: func returned\n");
}
void lkdtm_EXEC_USERSPACE(void)
--
2.31.1
[PATCH v2 09/13] lkdtm: Force do_nothing() out of line
LKDTM tests display that the run do_nothing() at a given
address, but in reality do_nothing() is inlined into the
caller.
Force it out of line so that it really runs text at the
displayed address.
Acked-by: Kees Cook
Signed-off-by: Christophe Leroy
---
drivers/misc/lkdtm/perms.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/misc/lkdtm/perms.c b/drivers/misc/lkdtm/perms.c
index 2dede2ef658f..60b3b2fe929d 100644
--- a/drivers/misc/lkdtm/perms.c
+++ b/drivers/misc/lkdtm/perms.c
@@ -21,7 +21,7 @@
/* This is non-const, so it will end up in the .data section. */
static u8 data_area[EXEC_SIZE];
-/* This is cost, so it will end up in the .rodata section. */
+/* This is const, so it will end up in the .rodata section. */
static const unsigned long rodata = 0xAA55AA55;
/* This is marked __ro_after_init, so it should ultimately be .rodata. */
@@ -31,7 +31,7 @@ static unsigned long ro_after_init __ro_after_init =
0x55AA5500;
* This just returns to the caller. It is designed to be copied into
* non-executable memory regions.
*/
-static void do_nothing(void)
+static noinline void do_nothing(void)
{
return;
}
--
2.31.1
