* [PATCH v1 2/8] powerpc/fsl_booke: Rename fsl_booke.c to fsl_book3e.c
From: Christophe Leroy @ 2021-10-15 10:02 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <d1ad9fdd9b27da3fdfa16510bb542ed51fa6e134.1634292136.git.christophe.leroy@csgroup.eu>
We have a myriad of CONFIG symbols around different variants
of BOOKEs, which would be worth tidying up one day.
But at least, make file names and CONFIG option match:
We have CONFIG_FSL_BOOKE and CONFIG_PPC_FSL_BOOK3E.
fsl_booke.c is selected by and only by CONFIG_PPC_FSL_BOOK3E.
So rename it fsl_book3e to reduce confusion.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/mm/nohash/Makefile | 4 ++--
arch/powerpc/mm/nohash/{fsl_booke.c => fsl_book3e.c} | 0
2 files changed, 2 insertions(+), 2 deletions(-)
rename arch/powerpc/mm/nohash/{fsl_booke.c => fsl_book3e.c} (100%)
diff --git a/arch/powerpc/mm/nohash/Makefile b/arch/powerpc/mm/nohash/Makefile
index 0424f6ce5bd8..b1f630d423d8 100644
--- a/arch/powerpc/mm/nohash/Makefile
+++ b/arch/powerpc/mm/nohash/Makefile
@@ -7,7 +7,7 @@ obj-$(CONFIG_PPC_BOOK3E_64) += tlb_low_64e.o book3e_pgtable.o
obj-$(CONFIG_40x) += 40x.o
obj-$(CONFIG_44x) += 44x.o
obj-$(CONFIG_PPC_8xx) += 8xx.o
-obj-$(CONFIG_PPC_FSL_BOOK3E) += fsl_booke.o
+obj-$(CONFIG_PPC_FSL_BOOK3E) += fsl_book3e.o
obj-$(CONFIG_RANDOMIZE_BASE) += kaslr_booke.o
ifdef CONFIG_HUGETLB_PAGE
obj-$(CONFIG_PPC_FSL_BOOK3E) += book3e_hugetlbpage.o
@@ -16,4 +16,4 @@ endif
# Disable kcov instrumentation on sensitive code
# This is necessary for booting with kcov enabled on book3e machines
KCOV_INSTRUMENT_tlb.o := n
-KCOV_INSTRUMENT_fsl_booke.o := n
+KCOV_INSTRUMENT_fsl_book3e.o := n
diff --git a/arch/powerpc/mm/nohash/fsl_booke.c b/arch/powerpc/mm/nohash/fsl_book3e.c
similarity index 100%
rename from arch/powerpc/mm/nohash/fsl_booke.c
rename to arch/powerpc/mm/nohash/fsl_book3e.c
--
2.31.1
^ permalink raw reply related
* [PATCH v1 6/8] powerpc/fsl_booke: Allocate separate TLBCAMs for readonly memory
From: Christophe Leroy @ 2021-10-15 10:02 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <d1ad9fdd9b27da3fdfa16510bb542ed51fa6e134.1634292136.git.christophe.leroy@csgroup.eu>
Reorganise TLBCAM allocation so that when STRICT_KERNEL_RWX is
enabled, TLBCAMs are allocated such that readonly memory uses
different TLBCAMs.
This results in an allocation looking like:
Memory CAM mapping: 4/4/4/1/1/1/1/16/16/16/64/64/64/256/256 Mb, residual: 256Mb
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/mm/nohash/fsl_book3e.c | 25 ++++++++++++++++++++++---
1 file changed, 22 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/mm/nohash/fsl_book3e.c b/arch/powerpc/mm/nohash/fsl_book3e.c
index 375b2b8238c1..c1bc11f46344 100644
--- a/arch/powerpc/mm/nohash/fsl_book3e.c
+++ b/arch/powerpc/mm/nohash/fsl_book3e.c
@@ -171,15 +171,34 @@ static unsigned long map_mem_in_cams_addr(phys_addr_t phys, unsigned long virt,
{
int i;
unsigned long amount_mapped = 0;
+ unsigned long boundary;
+
+ if (strict_kernel_rwx_enabled())
+ boundary = (unsigned long)(_sinittext - _stext);
+ else
+ boundary = ram;
/* Calculate CAM values */
- for (i = 0; ram && i < max_cam_idx; i++) {
+ for (i = 0; boundary && i < max_cam_idx; i++) {
+ unsigned long cam_sz;
+ pgprot_t prot = PAGE_KERNEL_X;
+
+ cam_sz = calc_cam_sz(boundary, virt, phys);
+ if (!dryrun)
+ settlbcam(i, virt, phys, cam_sz, pgprot_val(prot), 0);
+
+ boundary -= cam_sz;
+ amount_mapped += cam_sz;
+ virt += cam_sz;
+ phys += cam_sz;
+ }
+ for (ram -= amount_mapped; ram && i < max_cam_idx; i++) {
unsigned long cam_sz;
+ pgprot_t prot = PAGE_KERNEL_X;
cam_sz = calc_cam_sz(ram, virt, phys);
if (!dryrun)
- settlbcam(i, virt, phys, cam_sz,
- pgprot_val(PAGE_KERNEL_X), 0);
+ settlbcam(i, virt, phys, cam_sz, pgprot_val(prot), 0);
ram -= cam_sz;
amount_mapped += cam_sz;
--
2.31.1
^ permalink raw reply related
* [PATCH v1 3/8] powerpc/fsl_booke: Take exec flag into account when setting TLBCAMs
From: Christophe Leroy @ 2021-10-15 10:02 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <d1ad9fdd9b27da3fdfa16510bb542ed51fa6e134.1634292136.git.christophe.leroy@csgroup.eu>
Don't force MAS3_SX and MAS3_UX at all time. Take into account the
exec flag.
While at it, fix a couple of closeby style problems (indent with space
and unnecessary parenthesis), it keeps more readability.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/mm/nohash/fsl_book3e.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/mm/nohash/fsl_book3e.c b/arch/powerpc/mm/nohash/fsl_book3e.c
index 03dacbe940e5..fdf1029e62f0 100644
--- a/arch/powerpc/mm/nohash/fsl_book3e.c
+++ b/arch/powerpc/mm/nohash/fsl_book3e.c
@@ -122,15 +122,17 @@ static void settlbcam(int index, unsigned long virt, phys_addr_t phys,
TLBCAM[index].MAS2 |= (flags & _PAGE_GUARDED) ? MAS2_G : 0;
TLBCAM[index].MAS2 |= (flags & _PAGE_ENDIAN) ? MAS2_E : 0;
- TLBCAM[index].MAS3 = (phys & MAS3_RPN) | MAS3_SX | MAS3_SR;
- TLBCAM[index].MAS3 |= ((flags & _PAGE_RW) ? MAS3_SW : 0);
+ TLBCAM[index].MAS3 = (phys & MAS3_RPN) | MAS3_SR;
+ TLBCAM[index].MAS3 |= (flags & _PAGE_BAP_SX) ? MAS3_SX : 0;
+ TLBCAM[index].MAS3 |= (flags & _PAGE_RW) ? MAS3_SW : 0;
if (mmu_has_feature(MMU_FTR_BIG_PHYS))
TLBCAM[index].MAS7 = (u64)phys >> 32;
/* Below is unlikely -- only for large user pages or similar */
if (pte_user(__pte(flags))) {
- TLBCAM[index].MAS3 |= MAS3_UX | MAS3_UR;
- TLBCAM[index].MAS3 |= ((flags & _PAGE_RW) ? MAS3_UW : 0);
+ TLBCAM[index].MAS3 |= MAS3_UR;
+ TLBCAM[index].MAS3 |= (flags & _PAGE_EXEC) ? MAS3_UX : 0;
+ TLBCAM[index].MAS3 |= (flags & _PAGE_RW) ? MAS3_UW : 0;
}
tlbcam_addrs[index].start = virt;
--
2.31.1
^ permalink raw reply related
* Re: [PATCH v3 1/2] ftrace: disable preemption between ftrace_test_recursion_trylock/unlock()
From: 王贇 @ 2021-10-15 9:12 UTC (permalink / raw)
To: Petr Mladek
Cc: Peter Zijlstra (Intel), Paul Walmsley, James E.J. Bottomley,
Guo Ren, Jisheng Zhang, H. Peter Anvin, live-patching,
linux-riscv, Miroslav Benes, Paul Mackerras, Joe Lawrence,
Helge Deller, x86, linux-csky, Ingo Molnar, Albert Ou,
Jiri Kosina, Steven Rostedt, Borislav Petkov, Nicholas Piggin,
Josh Poimboeuf, Thomas Gleixner, linux-parisc, linux-kernel,
Palmer Dabbelt, Masami Hiramatsu, Colin Ian King, linuxppc-dev
In-Reply-To: <YWktujP6DcMnRIXT@alley>
On 2021/10/15 下午3:28, Petr Mladek wrote:
> On Fri 2021-10-15 11:13:08, 王贇 wrote:
>>
>>
>> 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.
>
> trace_test_and_set_recursion() does _not_ disable preemtion!
> It works only because all callers disable the preemption.
Yup.
>
> It means that the comment is wrong. It is not guarantted that the
> preemption will be disabled. It works only by chance.
>
>
>> 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
>
> How? Because it calls trace_test_and_set_recursion() via the trylock() API?
I mean currently all the direct caller of trace_test_and_set_recursion()
have disabled the preemption as you mentioned above, but yes if anyone later
write some kernel code to call trace_test_and_set_recursion() without
disabling preemption after, then promise broken.
>
>
>> , what uncovered is those users who call API trylock/unlock, isn't
>> it?
>
> And this is exactly the problem. trace_test_and_set_recursion() is in
> a public header. Anyone could use it. And if anyone uses it in the
> future without the trylock() and does not disable the preemtion
> explicitely then we are lost again.
>
> And it is even more dangerous. The original code disabled the
> preemtion on various layers. As a result, the preemtion was disabled
> several times for sure. It means that the deeper layers were
> always on the safe side.
>
> With this patch, if the first trace_test_and_set_recursion() caller
> does not disable preemtion then trylock() will not disable it either
> and the entire code is procceed with preemtion enabled.
Yes, what confusing me at first is that I think people who call
trace_test_and_set_recursion() without trylock() can only be a
ftrace hacker, not a user, but in case if anyone can use it without
respect to preemption stuff, then I think the logical should be inside
trace_test_and_set_recursion() rather than trylock().
>
>
>>> 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.
>
> I am sorry but it is still confusing. We need to find a better way
> how to clearly explain the difference between the safe and
> unsafe recursion.
>
> My understanding is that the recursion is:
>
> + "unsafe" when the trace code recursively enters the same trace point.
>
> + "safe" when ftrace_test_recursion_trylock() is called recursivelly
> while still processing the same trace entry.
Maybe take some example would be easier to understand...
Currently there are two way of using ftrace_test_recursion_trylock(),
one with TRACE_FTRACE_XXX we mark as A, one with TRACE_LIST_XXX we mark
as B, then:
A followed by B on same context got bit > 0
B followed by A on any context got bit 0
A followed by A on same context got bit > 0
A followed by A followed by A on same context got bit -1
B followed by B on same context got bit > 0
B followed by B followed by B on same context got bit -1
If we get rid of the TRACE_TRANSITION_BIT which allowed recursion for
onetime, then it would be:
A followed by B on same context got bit > 0
B followed by A on any context got bit 0
A followed by A on same context got bit -1
B followed by B on same context got bit -1
So as long as no continuously AAA it's safe?
>
>>>> +
>>>> + 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.
>
> Yes, it "should" not happen. My point is that we could make the API
> more safe. We could do the right thing when
> ftrace_test_recursion_unlock() is called with negative @bit.
> Ideally, we should also warn about the mis-use.
Agree with a WARN here on bit 0.
>
>
> Anyway, let's wait for Steven. It seems that he found another problem
> with the API that should be solved first. The fix will probably
> also help to better understand the "safe" vs "unsafe" recursion.
Cool~
Regards,
Michael Wang
>
> Best Regards,
> Petr
>
^ permalink raw reply
* Re: [PATCH v2 06/13] asm-generic: Use HAVE_FUNCTION_DESCRIPTORS to define associated stubs
From: Nicholas Piggin @ 2021-10-15 8:02 UTC (permalink / raw)
To: Andrew Morton, Arnd Bergmann, Benjamin Herrenschmidt,
Christophe Leroy, Helge Deller, Greg Kroah-Hartman,
James E.J. Bottomley, Kees Cook, Michael Ellerman, Paul Mackerras
Cc: linux-arch, linux-ia64, linux-parisc, linux-kernel, linux-mm,
linuxppc-dev
In-Reply-To: <7523a005-ea69-7c4c-64ad-bc2537921975@csgroup.eu>
Excerpts from Christophe Leroy's message of October 15, 2021 4:24 pm:
>
>
> Le 15/10/2021 à 08:16, Nicholas Piggin a écrit :
>> Excerpts from Christophe Leroy's message of October 14, 2021 3:49 pm:
>>> 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
>>
>> Is it worth putting this into Kconfig if you're going to
>> change it? In any case
>
> That was what I wanted to do in the begining but how can I do that in
> Kconfig ?
>
> #ifdef __powerpc64__
> #if defined(_CALL_ELF) && _CALL_ELF == 2
> #define PPC64_ELF_ABI_v2
> #else
> #define PPC64_ELF_ABI_v1
> #endif
> #endif /* __powerpc64__ */
>
> #ifdef PPC64_ELF_ABI_v1
> #define HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR 1
We have ELFv2 ABI / function descriptors iff big-endian so you could
just select based on that.
I have a patch that makes the ABI version configurable which cleans
some of this up a bit, but that can be rebased on your series if we
ever merge it. Maybe just add BUILD_BUG_ONs in the above ifdef block
to ensure CONFIG_HAVE_FUNCTION_DESCRIPTORS was set the right way, so
I don't forget.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH] powerpc/mce: check if event info is valid
From: Nicholas Piggin @ 2021-10-15 7:44 UTC (permalink / raw)
To: Ganesh, linuxppc-dev, Michael Ellerman; +Cc: mahesh
In-Reply-To: <87ily9nii0.fsf@mpe.ellerman.id.au>
Excerpts from Michael Ellerman's message of October 7, 2021 10:09 pm:
> Ganesh <ganeshgr@linux.ibm.com> writes:
>> On 8/6/21 6:53 PM, Ganesh Goudar wrote:
>>
>>> Check if the event info is valid before printing the
>>> event information. When a fwnmi enabled nested kvm guest
>>> hits a machine check exception L0 and L2 would generate
>>> machine check event info, But L1 would not generate any
>>> machine check event info as it won't go through 0x200
>>> vector and prints some unwanted message.
>>>
>>> To fix this, 'in_use' variable in machine check event info is
>>> no more in use, rename it to 'valid' and check if the event
>>> information is valid before logging the event information.
>>>
>>> without this patch L1 would print following message for
>>> exceptions encountered in L2, as event structure will be
>>> empty in L1.
>>>
>>> "Machine Check Exception, Unknown event version 0".
>>>
>>> Signed-off-by: Ganesh Goudar <ganeshgr@linux.ibm.com>
>>> ---
>>
>> Hi mpe, Any comments on this patch.
>
> The variable rename is a bit of a distraction.
>
> But ignoring that, how do we end up processing a machine_check_event
> that has in_use = 0?
>
> You don't give much detail on what call path you're talking about. I
> guess we're coming in via the calls in the KVM code?
>
> In the definition of kvm_vcpu_arch we have:
>
> struct machine_check_event mce_evt; /* Valid if trap == 0x200 */
>
> And you said we're not going via 0x200 in L1. But so shouldn't we be
> teaching the KVM code not to use mce_evt when trap is not 0x200?
I'm not sure we want the MCE to skip the L1 either. It should match the
L0 hypervisor behaviour as closely as reasonably possible.
We might have to teach the KVM pseries path to do something about the
0x200 before the common HV guest exit handler (which is where the L1
message comes from).
Thanks,
Nick
^ permalink raw reply
* [PATCH 2/2] powerpc/eeh: Use a goto for recovery failures
From: Daniel Axtens @ 2021-10-15 7:06 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Mahesh Salgaonkar, oohall, Daniel Axtens
In-Reply-To: <20211015070628.1331635-1-dja@axtens.net>
From: Oliver O'Halloran <oohall@gmail.com>
The EEH recovery logic in eeh_handle_normal_event() has some pretty strange
flow control. If we remove all the actual recovery logic we're left with
the following skeleton:
if (result != PCI_ERS_RESULT_DISCONNECT) {
...
}
if (result != PCI_ERS_RESULT_DISCONNECT) {
...
}
if (result == PCI_ERS_RESULT_NONE) {
...
}
if (result == PCI_ERS_RESULT_CAN_RECOVER) {
...
}
if (result == PCI_ERS_RESULT_CAN_RECOVER) {
...
}
if (result == PCI_ERS_RESULT_NEED_RESET) {
...
}
if ((result == PCI_ERS_RESULT_RECOVERED) ||
(result == PCI_ERS_RESULT_NONE)) {
...
goto out;
}
/*
* unsuccessful recovery / PCI_ERS_RESULT_DISCONECTED
* handling is here.
*/
...
out:
...
Most of the "if () { ... }" blocks above change "result" to
PCI_ERS_RESULT_DISCONNECTED if an error occurs in that recovery step. This
makes the control flow a bit confusing since it breaks the early-exit
pattern that is generally used in Linux. In any case we end up handling the
error in the final else block so why not just jump there directly? Doing so
also allows us to de-indent a bunch of code.
No functional changes.
Cc: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
[dja: rebase on top of linux-next + my preceeding refactor,
move clearing the EEH_DEV_NO_HANDLER bit above the first goto so that
it is always clear in the error handler code as it was before.]
Signed-off-by: Daniel Axtens <dja@axtens.net>
---
arch/powerpc/kernel/eeh_driver.c | 93 ++++++++++++++++----------------
1 file changed, 45 insertions(+), 48 deletions(-)
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index cb3ac555c944..b8d5805c446a 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -905,18 +905,19 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
}
#endif /* CONFIG_STACKTRACE */
+ eeh_for_each_pe(pe, tmp_pe)
+ eeh_pe_for_each_dev(tmp_pe, edev, tmp)
+ edev->mode &= ~EEH_DEV_NO_HANDLER;
+
eeh_pe_update_time_stamp(pe);
pe->freeze_count++;
if (pe->freeze_count > eeh_max_freezes) {
pr_err("EEH: PHB#%x-PE#%x has failed %d times in the last hour and has been permanently disabled.\n",
pe->phb->global_number, pe->addr,
pe->freeze_count);
- result = PCI_ERS_RESULT_DISCONNECT;
- }
- eeh_for_each_pe(pe, tmp_pe)
- eeh_pe_for_each_dev(tmp_pe, edev, tmp)
- edev->mode &= ~EEH_DEV_NO_HANDLER;
+ goto recover_failed;
+ }
/* Walk the various device drivers attached to this slot through
* a reset sequence, giving each an opportunity to do what it needs
@@ -928,39 +929,38 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
* the error. Override the result if necessary to have partially
* hotplug for this case.
*/
- if (result != PCI_ERS_RESULT_DISCONNECT) {
- pr_warn("EEH: This PCI device has failed %d times in the last hour and will be permanently disabled after %d failures.\n",
- pe->freeze_count, eeh_max_freezes);
- pr_info("EEH: Notify device drivers to shutdown\n");
- eeh_set_channel_state(pe, pci_channel_io_frozen);
- eeh_set_irq_state(pe, false);
- eeh_pe_report("error_detected(IO frozen)", pe,
- eeh_report_error, &result);
- if ((pe->type & EEH_PE_PHB) &&
- result != PCI_ERS_RESULT_NONE &&
- result != PCI_ERS_RESULT_NEED_RESET)
- result = PCI_ERS_RESULT_NEED_RESET;
- }
+ pr_warn("EEH: This PCI device has failed %d times in the last hour and will be permanently disabled after %d failures.\n",
+ pe->freeze_count, eeh_max_freezes);
+ pr_info("EEH: Notify device drivers to shutdown\n");
+ eeh_set_channel_state(pe, pci_channel_io_frozen);
+ eeh_set_irq_state(pe, false);
+ eeh_pe_report("error_detected(IO frozen)", pe,
+ eeh_report_error, &result);
+ if (result == PCI_ERS_RESULT_DISCONNECT)
+ goto recover_failed;
+
+ /*
+ * Error logged on a PHB are always fences which need a full
+ * PHB reset to clear so force that to happen.
+ */
+ if ((pe->type & EEH_PE_PHB) && result != PCI_ERS_RESULT_NONE)
+ result = PCI_ERS_RESULT_NEED_RESET;
/* Get the current PCI slot state. This can take a long time,
* sometimes over 300 seconds for certain systems.
*/
- if (result != PCI_ERS_RESULT_DISCONNECT) {
- rc = eeh_wait_state(pe, MAX_WAIT_FOR_RECOVERY*1000);
- if (rc < 0 || rc == EEH_STATE_NOT_SUPPORT) {
- pr_warn("EEH: Permanent failure\n");
- result = PCI_ERS_RESULT_DISCONNECT;
- }
+ rc = eeh_wait_state(pe, MAX_WAIT_FOR_RECOVERY * 1000);
+ if (rc < 0 || rc == EEH_STATE_NOT_SUPPORT) {
+ pr_warn("EEH: Permanent failure\n");
+ goto recover_failed;
}
/* Since rtas may enable MMIO when posting the error log,
* don't post the error log until after all dev drivers
* have been informed.
*/
- if (result != PCI_ERS_RESULT_DISCONNECT) {
- pr_info("EEH: Collect temporary log\n");
- eeh_slot_error_detail(pe, EEH_LOG_TEMP);
- }
+ pr_info("EEH: Collect temporary log\n");
+ eeh_slot_error_detail(pe, EEH_LOG_TEMP);
/* If all device drivers were EEH-unaware, then shut
* down all of the device drivers, and hope they
@@ -970,9 +970,8 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
pr_info("EEH: Reset with hotplug activity\n");
rc = eeh_reset_device(pe, bus, NULL, false);
if (rc) {
- pr_warn("%s: Unable to reset, err=%d\n",
- __func__, rc);
- result = PCI_ERS_RESULT_DISCONNECT;
+ pr_warn("%s: Unable to reset, err=%d\n", __func__, rc);
+ goto recover_failed;
}
}
@@ -980,10 +979,10 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
if (result == PCI_ERS_RESULT_CAN_RECOVER) {
pr_info("EEH: Enable I/O for affected devices\n");
rc = eeh_pci_enable(pe, EEH_OPT_THAW_MMIO);
+ if (rc < 0)
+ goto recover_failed;
- if (rc < 0) {
- result = PCI_ERS_RESULT_DISCONNECT;
- } else if (rc) {
+ if (rc) {
result = PCI_ERS_RESULT_NEED_RESET;
} else {
pr_info("EEH: Notify device drivers to resume I/O\n");
@@ -991,15 +990,13 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
eeh_report_mmio_enabled, &result);
}
}
-
- /* If all devices reported they can proceed, then re-enable DMA */
if (result == PCI_ERS_RESULT_CAN_RECOVER) {
pr_info("EEH: Enabled DMA for affected devices\n");
rc = eeh_pci_enable(pe, EEH_OPT_THAW_DMA);
+ if (rc < 0)
+ goto recover_failed;
- if (rc < 0) {
- result = PCI_ERS_RESULT_DISCONNECT;
- } else if (rc) {
+ if (rc) {
result = PCI_ERS_RESULT_NEED_RESET;
} else {
/*
@@ -1017,16 +1014,15 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
pr_info("EEH: Reset without hotplug activity\n");
rc = eeh_reset_device(pe, bus, &rmv_data, true);
if (rc) {
- pr_warn("%s: Cannot reset, err=%d\n",
- __func__, rc);
- result = PCI_ERS_RESULT_DISCONNECT;
- } else {
- result = PCI_ERS_RESULT_NONE;
- eeh_set_channel_state(pe, pci_channel_io_normal);
- eeh_set_irq_state(pe, true);
- eeh_pe_report("slot_reset", pe, eeh_report_reset,
- &result);
+ pr_warn("%s: Cannot reset, err=%d\n", __func__, rc);
+ goto recover_failed;
}
+
+ result = PCI_ERS_RESULT_NONE;
+ eeh_set_channel_state(pe, pci_channel_io_normal);
+ eeh_set_irq_state(pe, true);
+ eeh_pe_report("slot_reset", pe, eeh_report_reset,
+ &result);
}
if ((result == PCI_ERS_RESULT_RECOVERED) ||
@@ -1057,6 +1053,7 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
goto out;
}
+recover_failed:
/*
* About 90% of all real-life EEH failures in the field
* are due to poorly seated PCI cards. Only 10% or so are
--
2.25.1
^ permalink raw reply related
* [PATCH 1/2] eeh: Small refactor of eeh_handle_normal_event
From: Daniel Axtens @ 2021-10-15 7:06 UTC (permalink / raw)
To: linuxppc-dev; +Cc: oohall, Daniel Axtens
The control flow of eeh_handle_normal_event is a bit tricky.
Break out one of the error handling paths - rather than be in
an else block, we'll make it part of the regular body of the
function and put a 'goto out;' in the true limb of the if.
Signed-off-by: Daniel Axtens <dja@axtens.net>
---
This doesn't really make things that much simpler to comprehend
by itself but it makes the next patch a bit cleaner.
---
arch/powerpc/kernel/eeh_driver.c | 69 ++++++++++++++++----------------
1 file changed, 35 insertions(+), 34 deletions(-)
diff --git a/arch/powerpc/kernel/eeh_driver.c b/arch/powerpc/kernel/eeh_driver.c
index 3eff6a4888e7..cb3ac555c944 100644
--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c
@@ -1054,45 +1054,46 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
}
pr_info("EEH: Recovery successful.\n");
- } else {
- /*
- * About 90% of all real-life EEH failures in the field
- * are due to poorly seated PCI cards. Only 10% or so are
- * due to actual, failed cards.
- */
- pr_err("EEH: Unable to recover from failure from PHB#%x-PE#%x.\n"
- "Please try reseating or replacing it\n",
- pe->phb->global_number, pe->addr);
+ goto out;
+ }
- eeh_slot_error_detail(pe, EEH_LOG_PERM);
+ /*
+ * About 90% of all real-life EEH failures in the field
+ * are due to poorly seated PCI cards. Only 10% or so are
+ * due to actual, failed cards.
+ */
+ pr_err("EEH: Unable to recover from failure from PHB#%x-PE#%x.\n"
+ "Please try reseating or replacing it\n",
+ pe->phb->global_number, pe->addr);
- /* Notify all devices that they're about to go down. */
- eeh_set_channel_state(pe, pci_channel_io_perm_failure);
- eeh_set_irq_state(pe, false);
- eeh_pe_report("error_detected(permanent failure)", pe,
- eeh_report_failure, NULL);
+ eeh_slot_error_detail(pe, EEH_LOG_PERM);
- /* Mark the PE to be removed permanently */
- eeh_pe_state_mark(pe, EEH_PE_REMOVED);
+ /* Notify all devices that they're about to go down. */
+ eeh_set_channel_state(pe, pci_channel_io_perm_failure);
+ eeh_set_irq_state(pe, false);
+ eeh_pe_report("error_detected(permanent failure)", pe,
+ eeh_report_failure, NULL);
- /*
- * Shut down the device drivers for good. We mark
- * all removed devices correctly to avoid access
- * the their PCI config any more.
- */
- if (pe->type & EEH_PE_VF) {
- eeh_pe_dev_traverse(pe, eeh_rmv_device, NULL);
- eeh_pe_dev_mode_mark(pe, EEH_DEV_REMOVED);
- } else {
- eeh_pe_state_clear(pe, EEH_PE_PRI_BUS, true);
- eeh_pe_dev_mode_mark(pe, EEH_DEV_REMOVED);
+ /* Mark the PE to be removed permanently */
+ eeh_pe_state_mark(pe, EEH_PE_REMOVED);
- pci_lock_rescan_remove();
- pci_hp_remove_devices(bus);
- pci_unlock_rescan_remove();
- /* The passed PE should no longer be used */
- return;
- }
+ /*
+ * Shut down the device drivers for good. We mark
+ * all removed devices correctly to avoid access
+ * the their PCI config any more.
+ */
+ if (pe->type & EEH_PE_VF) {
+ eeh_pe_dev_traverse(pe, eeh_rmv_device, NULL);
+ eeh_pe_dev_mode_mark(pe, EEH_DEV_REMOVED);
+ } else {
+ eeh_pe_state_clear(pe, EEH_PE_PRI_BUS, true);
+ eeh_pe_dev_mode_mark(pe, EEH_DEV_REMOVED);
+
+ pci_lock_rescan_remove();
+ pci_hp_remove_devices(bus);
+ pci_unlock_rescan_remove();
+ /* The passed PE should no longer be used */
+ return;
}
out:
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v2 08/13] asm-generic: Refactor dereference_[kernel]_function_descriptor()
From: Nicholas Piggin @ 2021-10-15 7:00 UTC (permalink / raw)
To: Andrew Morton, Arnd Bergmann, Benjamin Herrenschmidt,
Christophe Leroy, Helge Deller, Greg Kroah-Hartman,
James E.J. Bottomley, Kees Cook, Michael Ellerman, Paul Mackerras
Cc: linux-arch, linux-ia64, linux-parisc, linux-kernel, linux-mm,
linuxppc-dev
In-Reply-To: <865b5c872814e3291fe7afabcc110f53b3457b56.1634190022.git.christophe.leroy@csgroup.eu>
Excerpts from Christophe Leroy's message of October 14, 2021 3:49 pm:
> 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.
We should be moving more stuff out of extable.c (including all the
kernel address tests). lib/kimage.c or kelf.c or something.
It could be after your series though.
>
> Reviewed-by: Kees Cook <keescook@chromium.org>
> Reviewed-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> 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 *)&desc->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 *)&desc->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 *)&desc->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 <linux/elf.h>
> #include <linux/ftrace.h>
> #include <linux/memory.h>
> #include <linux/extable.h>
> @@ -159,12 +160,32 @@ int kernel_text_address(unsigned long addr)
> }
>
> /*
> - * On some architectures (PPC64, IA64) function pointers
> + * On some architectures (PPC64, IA64, PARISC) function pointers
> * are actually only tokens to some data that then holds the
> * real function address. As a result, to find if a function
> * pointer is part of the kernel text, we need to do some
> * special dereferencing first.
> */
> +#ifdef HAVE_FUNCTION_DESCRIPTORS
> +void *dereference_function_descriptor(void *ptr)
> +{
> + func_desc_t *desc = ptr;
> + void *p;
> +
> + if (!get_kernel_nofault(p, (void *)&desc->addr))
> + ptr = p;
I know you're just copying existing code. This seems a bit risky though.
I don't think anything good could come of just treating the descriptor
address like a function entry address if we failed to load from it for
whatever reason.
Existing callers might be benign but the API is not good. It should
give a nice fail return or BUG. If we change that then we should also
change the name and pass the correct type to it too.
Thanks,
Nick
^ permalink raw reply
* [PATCH] macintosh: ams: replace snprintf in show functions with sysfs_emit
From: Qing Wang @ 2021-10-15 6:49 UTC (permalink / raw)
To: Benjamin Herrenschmidt, linuxppc-dev, linux-kernel; +Cc: Qing Wang
show() must not use snprintf() when formatting the value to be
returned to user space.
Fix the following coccicheck warning:
drivers/macintosh/ams/ams-core.c:53: WARNING: use scnprintf or sprintf.
Use sysfs_emit instead of scnprintf or sprintf makes more sense.
Signed-off-by: Qing Wang <wangqing@vivo.com>
---
drivers/macintosh/ams/ams-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/macintosh/ams/ams-core.c b/drivers/macintosh/ams/ams-core.c
index 01eeb23..877e8cb 100644
--- a/drivers/macintosh/ams/ams-core.c
+++ b/drivers/macintosh/ams/ams-core.c
@@ -50,7 +50,7 @@ static ssize_t ams_show_current(struct device *dev,
ams_sensors(&x, &y, &z);
mutex_unlock(&ams_info.lock);
- return snprintf(buf, PAGE_SIZE, "%d %d %d\n", x, y, z);
+ return sysfs_emit(buf, "%d %d %d\n", x, y, z);
}
static DEVICE_ATTR(current, S_IRUGO, ams_show_current, NULL);
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v2 06/13] asm-generic: Use HAVE_FUNCTION_DESCRIPTORS to define associated stubs
From: Christophe Leroy @ 2021-10-15 6:24 UTC (permalink / raw)
To: Nicholas Piggin, Andrew Morton, Arnd Bergmann,
Benjamin Herrenschmidt, Helge Deller, Greg Kroah-Hartman,
James E.J. Bottomley, Kees Cook, Michael Ellerman, Paul Mackerras
Cc: linux-arch, linux-ia64, linux-parisc, linux-kernel, linux-mm,
linuxppc-dev
In-Reply-To: <1634278340.5yp7xtm7um.astroid@bobo.none>
Le 15/10/2021 à 08:16, Nicholas Piggin a écrit :
> Excerpts from Christophe Leroy's message of October 14, 2021 3:49 pm:
>> 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
>
> Is it worth putting this into Kconfig if you're going to
> change it? In any case
That was what I wanted to do in the begining but how can I do that in
Kconfig ?
#ifdef __powerpc64__
#if defined(_CALL_ELF) && _CALL_ELF == 2
#define PPC64_ELF_ABI_v2
#else
#define PPC64_ELF_ABI_v1
#endif
#endif /* __powerpc64__ */
#ifdef PPC64_ELF_ABI_v1
#define HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR 1
Christophe
>
> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
>
>>
>> Reviewed-by: Kees Cook <keescook@chromium.org>
>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>> ---
>> 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 <linux/elf.h>
>> #include <linux/uaccess.h>
>> +
>> +#define HAVE_FUNCTION_DESCRIPTORS 1
>> +
>> #include <asm-generic/sections.h>
>>
>> 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 <asm-generic/sections.h>
>>
>> @@ -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 <asm-generic/sections.h>
>>
>> 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
>>
>>
>>
^ permalink raw reply
* Re: [PATCH v2 06/13] asm-generic: Use HAVE_FUNCTION_DESCRIPTORS to define associated stubs
From: Nicholas Piggin @ 2021-10-15 6:16 UTC (permalink / raw)
To: Andrew Morton, Arnd Bergmann, Benjamin Herrenschmidt,
Christophe Leroy, Helge Deller, Greg Kroah-Hartman,
James E.J. Bottomley, Kees Cook, Michael Ellerman, Paul Mackerras
Cc: linux-arch, linux-ia64, linux-parisc, linux-kernel, linux-mm,
linuxppc-dev
In-Reply-To: <4fda65cda906e56aa87806b658e0828c64792403.1634190022.git.christophe.leroy@csgroup.eu>
Excerpts from Christophe Leroy's message of October 14, 2021 3:49 pm:
> 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
Is it worth putting this into Kconfig if you're going to
change it? In any case
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
>
> Reviewed-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> 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 <linux/elf.h>
> #include <linux/uaccess.h>
> +
> +#define HAVE_FUNCTION_DESCRIPTORS 1
> +
> #include <asm-generic/sections.h>
>
> 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 <asm-generic/sections.h>
>
> @@ -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 <asm-generic/sections.h>
>
> 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
>
>
>
^ permalink raw reply
* Re: [PATCH v2 03/13] powerpc: Remove func_descr_t
From: Nicholas Piggin @ 2021-10-15 6:11 UTC (permalink / raw)
To: Andrew Morton, Arnd Bergmann, Benjamin Herrenschmidt,
Christophe Leroy, Helge Deller, Daniel Axtens, Greg Kroah-Hartman,
James E.J. Bottomley, Kees Cook, Michael Ellerman, Paul Mackerras
Cc: linux-arch, linux-ia64, linux-parisc, linux-kernel, linux-mm,
linuxppc-dev
In-Reply-To: <b02d5211-2f00-b303-766b-d612c1bd4402@csgroup.eu>
Excerpts from Christophe Leroy's message of October 15, 2021 3:19 pm:
>
>
> Le 15/10/2021 à 00:17, Daniel Axtens a écrit :
>> Christophe Leroy <christophe.leroy@csgroup.eu> 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;
If you make a generic data type for architectures without function
descriptors as such
typedef struct func_desc {
char addr[0];
} func_desc_t;
Then you can do that with no if. The downside is your addr has to be
char * and it's maybe not helpful to be so "clever".
>> - 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.
I would prefer the func_descr_t (with 'toc' and 'env') if you're going
to change it.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH v2 02/13] powerpc: Rename 'funcaddr' to 'addr' in 'struct ppc64_opd_entry'
From: Nicholas Piggin @ 2021-10-15 6:01 UTC (permalink / raw)
To: Andrew Morton, Arnd Bergmann, Benjamin Herrenschmidt,
Christophe Leroy, Helge Deller, Greg Kroah-Hartman,
James E.J. Bottomley, Kees Cook, Michael Ellerman, Paul Mackerras
Cc: linux-arch, linux-ia64, linux-parisc, linux-kernel, linux-mm,
linuxppc-dev
In-Reply-To: <49f59a8bf2c4d95cfaa03bd3dd3c1569822ad6ba.1634190022.git.christophe.leroy@csgroup.eu>
Excerpts from Christophe Leroy's message of October 14, 2021 3:49 pm:
> 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.
It is the "address of the entry point of the function" according to
powerpc ELF spec, so addr seems fine.
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
>
> Reviewed-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> 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 *)&desc->funcaddr))
> + if (!get_kernel_nofault(p, (void *)&desc->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
>
>
>
^ permalink raw reply
* Re: [PATCH v2 01/13] powerpc: Move 'struct ppc64_opd_entry' back into asm/elf.h
From: Nicholas Piggin @ 2021-10-15 5:57 UTC (permalink / raw)
To: Andrew Morton, Arnd Bergmann, Benjamin Herrenschmidt,
Christophe Leroy, Helge Deller, Greg Kroah-Hartman,
James E.J. Bottomley, Kees Cook, Michael Ellerman, Paul Mackerras
Cc: linux-arch, linux-ia64, linux-parisc, linux-kernel, linux-mm,
linuxppc-dev
In-Reply-To: <42d2a571677e60082c0a5b3e52e855aa58c0b1fc.1634190022.git.christophe.leroy@csgroup.eu>
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 <keescook@chromium.org>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> 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 <npiggin@gmail.com>
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
^ permalink raw reply
* [powerpc:next] BUILD SUCCESS 8f6aca0e0f26eaaee670cd27896993a45cdc8f9e
From: kernel test robot @ 2021-10-15 5:36 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
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
arm64 allyesconfig
arm64 defconfig
arm allyesconfig
arm allmodconfig
i386 randconfig-c001-20211014
mips loongson1b_defconfig
arm mmp2_defconfig
mips rb532_defconfig
arm exynos_defconfig
sh edosk7705_defconfig
arm zeus_defconfig
sh se7206_defconfig
sh ap325rxa_defconfig
powerpc socrates_defconfig
powerpc ebony_defconfig
powerpc kmeter1_defconfig
s390 zfcpdump_defconfig
mips qi_lb60_defconfig
arm colibri_pxa270_defconfig
powerpc tqm8560_defconfig
arm qcom_defconfig
arc axs103_smp_defconfig
powerpc currituck_defconfig
mips decstation_64_defconfig
powerpc skiroot_defconfig
xtensa virt_defconfig
mips bigsur_defconfig
arc alldefconfig
m68k q40_defconfig
sh edosk7760_defconfig
mips malta_qemu_32r6_defconfig
powerpc lite5200b_defconfig
arm orion5x_defconfig
mips workpad_defconfig
mips loongson2k_defconfig
arc haps_hs_smp_defconfig
m68k atari_defconfig
um defconfig
sh rts7751r2d1_defconfig
arc defconfig
riscv alldefconfig
powerpc ppc40x_defconfig
m68k sun3_defconfig
m68k stmark2_defconfig
powerpc tqm8xx_defconfig
powerpc adder875_defconfig
arc nsim_700_defconfig
powerpc tqm8541_defconfig
nios2 3c120_defconfig
mips fuloong2e_defconfig
powerpc warp_defconfig
openrisc or1ksim_defconfig
arm gemini_defconfig
powerpc pasemi_defconfig
m68k apollo_defconfig
arm imx_v6_v7_defconfig
sh migor_defconfig
powerpc stx_gp3_defconfig
arm colibri_pxa300_defconfig
s390 debug_defconfig
sh j2_defconfig
sh espt_defconfig
mips bcm47xx_defconfig
powerpc mpc8560_ads_defconfig
arm vexpress_defconfig
powerpc pmac32_defconfig
powerpc mpc8315_rdb_defconfig
powerpc ge_imp3a_defconfig
powerpc sequoia_defconfig
m68k hp300_defconfig
sh rsk7269_defconfig
ia64 defconfig
powerpc arches_defconfig
riscv rv32_defconfig
sh sh7757lcr_defconfig
arm randconfig-c002-20211014
x86_64 randconfig-c001-20211014
ia64 allmodconfig
ia64 allyesconfig
m68k allmodconfig
m68k defconfig
m68k allyesconfig
nios2 defconfig
nds32 allnoconfig
arc allyesconfig
nds32 defconfig
nios2 allyesconfig
csky defconfig
alpha defconfig
alpha allyesconfig
xtensa allyesconfig
h8300 allyesconfig
sh allmodconfig
parisc defconfig
s390 defconfig
s390 allyesconfig
s390 allmodconfig
parisc allyesconfig
sparc allyesconfig
sparc defconfig
i386 defconfig
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
riscv nommu_k210_defconfig
riscv allyesconfig
riscv nommu_virt_defconfig
riscv allnoconfig
riscv defconfig
riscv allmodconfig
x86_64 rhel-8.3-kselftests
um x86_64_defconfig
um i386_defconfig
x86_64 allyesconfig
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
riscv randconfig-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 randconfig-r042-20211014
hexagon randconfig-r045-20211014
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* Re: [PATCH v2 03/13] powerpc: Remove func_descr_t
From: Christophe Leroy @ 2021-10-15 5:19 UTC (permalink / raw)
To: Daniel Axtens, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, Andrew Morton, James E.J. Bottomley,
Helge Deller, Arnd Bergmann, Kees Cook, Greg Kroah-Hartman
Cc: linux-arch, linux-ia64, linux-parisc, linux-kernel, linux-mm,
linuxppc-dev
In-Reply-To: <874k9j45fm.fsf@dja-thinkpad.axtens.net>
Le 15/10/2021 à 00:17, Daniel Axtens a écrit :
> Christophe Leroy <christophe.leroy@csgroup.eu> 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 <christophe.leroy@csgroup.eu>
>> ---
>> 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, &funct_desc_ptr->entry);
>> - err |= get_user(regs->gpr[2], &funct_desc_ptr->toc);
>> + err |= get_user(regs->ctr, &funct_desc_ptr->addr);
>> + err |= get_user(regs->gpr[2], &funct_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
^ permalink raw reply
* Re: [PATCH v2 02/13] powerpc: Rename 'funcaddr' to 'addr' in 'struct ppc64_opd_entry'
From: Christophe Leroy @ 2021-10-15 4:59 UTC (permalink / raw)
To: Daniel Axtens, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman, Andrew Morton, James E.J. Bottomley,
Helge Deller, Arnd Bergmann, Kees Cook, Greg Kroah-Hartman
Cc: linux-arch, linux-ia64, linux-parisc, linux-kernel, linux-mm,
linuxppc-dev
In-Reply-To: <877def46xc.fsf@dja-thinkpad.axtens.net>
Le 14/10/2021 à 23:45, Daniel Axtens a écrit :
> Christophe Leroy <christophe.leroy@csgroup.eu> 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 <dja@axtens.net>
>
> Kind regards,
> Daniel
>
>> Reviewed-by: Kees Cook <keescook@chromium.org>
>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>> ---
>> 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 *)&desc->funcaddr))
>> + if (!get_kernel_nofault(p, (void *)&desc->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
^ permalink raw reply
* [PATCH][next] powerpc/vas: Fix potential NULL pointer dereference
From: Gustavo A. R. Silva @ 2021-10-15 5:03 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
Nicholas Piggin, Haren Myneni
Cc: linux-hardening, linuxppc-dev, linux-kernel, Gustavo A. R. Silva
(!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: stable@vger.kernel.org
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
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
^ permalink raw reply related
* Re: [PATCH v3 1/2] ftrace: disable preemption between ftrace_test_recursion_trylock/unlock()
From: 王贇 @ 2021-10-15 4:45 UTC (permalink / raw)
To: Petr Mladek
Cc: Peter Zijlstra (Intel), Paul Walmsley, James E.J. Bottomley,
Guo Ren, Jisheng Zhang, H. Peter Anvin, live-patching,
linux-riscv, Miroslav Benes, Paul Mackerras, Joe Lawrence,
Helge Deller, x86, linux-csky, Ingo Molnar, Albert Ou,
Jiri Kosina, Steven Rostedt, Borislav Petkov, Nicholas Piggin,
Josh Poimboeuf, Thomas Gleixner, linux-parisc, linux-kernel,
Palmer Dabbelt, Masami Hiramatsu, Colin Ian King, linuxppc-dev
In-Reply-To: <5e907ed3-806b-b0e5-518d-d2f3b265377f@linux.alibaba.com>
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;
>
>>
>>
^ permalink raw reply
* Re: [PATCH] ibmvscsi: use GFP_KERNEL with dma_alloc_coherent in initialize_event_pool
From: Michael Ellerman @ 2021-10-15 4:36 UTC (permalink / raw)
To: Tyrel Datwyler, tyreld
Cc: brking, james.bottomley, linuxppc-dev, linux-scsi,
martin.petersen
In-Reply-To: <bbab1043-ee3a-6d5b-7ff5-ea5ed84e9fb8@linux.ibm.com>
Tyrel Datwyler <tyreld@linux.ibm.com> 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-tyreld@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
^ permalink raw reply
* [powerpc:next-test] BUILD SUCCESS 3091f5fc5f1df7741ddf326561384e0997eca2a1
From: kernel test robot @ 2021-10-15 3:52 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next-test
branch HEAD: 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
arm64 allyesconfig
arm64 defconfig
arm allyesconfig
arm allmodconfig
i386 randconfig-c001-20211014
mips loongson1b_defconfig
arm mmp2_defconfig
mips rb532_defconfig
arm exynos_defconfig
sh edosk7705_defconfig
arm zeus_defconfig
sh se7206_defconfig
sh ap325rxa_defconfig
s390 zfcpdump_defconfig
mips qi_lb60_defconfig
arm colibri_pxa270_defconfig
powerpc tqm8560_defconfig
arm qcom_defconfig
arc axs103_smp_defconfig
powerpc currituck_defconfig
mips decstation_64_defconfig
powerpc skiroot_defconfig
xtensa virt_defconfig
mips bigsur_defconfig
arc alldefconfig
mips workpad_defconfig
mips loongson2k_defconfig
arc haps_hs_smp_defconfig
m68k atari_defconfig
um defconfig
powerpc tqm8xx_defconfig
powerpc adder875_defconfig
arc nsim_700_defconfig
powerpc tqm8541_defconfig
nios2 3c120_defconfig
mips fuloong2e_defconfig
powerpc lite5200b_defconfig
powerpc warp_defconfig
openrisc or1ksim_defconfig
powerpc mpc8560_ads_defconfig
arm vexpress_defconfig
powerpc pmac32_defconfig
powerpc mpc8315_rdb_defconfig
powerpc ge_imp3a_defconfig
powerpc sequoia_defconfig
arm randconfig-c002-20211014
x86_64 randconfig-c001-20211014
ia64 allmodconfig
ia64 defconfig
ia64 allyesconfig
m68k allmodconfig
m68k defconfig
m68k allyesconfig
nios2 defconfig
arc allyesconfig
nds32 allnoconfig
nds32 defconfig
nios2 allyesconfig
csky defconfig
alpha defconfig
alpha allyesconfig
h8300 allyesconfig
arc defconfig
sh allmodconfig
xtensa allyesconfig
parisc defconfig
s390 defconfig
s390 allyesconfig
s390 allmodconfig
parisc allyesconfig
sparc allyesconfig
sparc defconfig
i386 defconfig
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
riscv nommu_k210_defconfig
riscv allyesconfig
riscv nommu_virt_defconfig
riscv allnoconfig
riscv defconfig
riscv rv32_defconfig
riscv allmodconfig
x86_64 rhel-8.3-kselftests
um x86_64_defconfig
um i386_defconfig
x86_64 defconfig
x86_64 rhel-8.3
x86_64 kexec
x86_64 allyesconfig
clang tested configs:
arm randconfig-c002-20211014
i386 randconfig-c001-20211014
s390 randconfig-c005-20211014
x86_64 randconfig-c007-20211014
powerpc randconfig-c003-20211014
riscv randconfig-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 randconfig-r042-20211014
hexagon randconfig-r045-20211014
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* Re: [PATCH v3 1/2] ftrace: disable preemption between ftrace_test_recursion_trylock/unlock()
From: Steven Rostedt @ 2021-10-15 3:39 UTC (permalink / raw)
To: Petr Mladek
Cc: 王贇, Peter Zijlstra (Intel), Paul Walmsley,
James E.J. Bottomley, Guo Ren, Jisheng Zhang, H. Peter Anvin,
live-patching, linux-riscv, Miroslav Benes, Paul Mackerras,
Joe Lawrence, Helge Deller, x86, linux-csky, Ingo Molnar,
Albert Ou, Jiri Kosina, Nicholas Piggin, Borislav Petkov,
Josh Poimboeuf, Thomas Gleixner, linux-parisc, linux-kernel,
Palmer Dabbelt, Masami Hiramatsu, Colin Ian King, linuxppc-dev
In-Reply-To: <YWhJP41cNwDphYsv@alley>
On Thu, 14 Oct 2021 17:14:07 +0200
Petr Mladek <pmladek@suse.com> 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
^ permalink raw reply
* Re: [PATCH v3 1/2] ftrace: disable preemption between ftrace_test_recursion_trylock/unlock()
From: 王贇 @ 2021-10-15 3:13 UTC (permalink / raw)
To: Petr Mladek
Cc: Peter Zijlstra (Intel), Paul Walmsley, James E.J. Bottomley,
Guo Ren, Jisheng Zhang, H. Peter Anvin, live-patching,
linux-riscv, Miroslav Benes, Paul Mackerras, Joe Lawrence,
Helge Deller, x86, linux-csky, Ingo Molnar, Albert Ou,
Jiri Kosina, Steven Rostedt, Borislav Petkov, Nicholas Piggin,
Josh Poimboeuf, Thomas Gleixner, linux-parisc, linux-kernel,
Palmer Dabbelt, Masami Hiramatsu, Colin Ian King, linuxppc-dev
In-Reply-To: <YWhJP41cNwDphYsv@alley>
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 <pmladek@suse.com>
> 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 <pmladek@suse.com>
> ---
> 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;
>
>
^ permalink raw reply
* [powerpc:merge] BUILD SUCCESS 38947529bb05bbb8acfb2fe0ff96c2f1bc3f2c96
From: kernel test robot @ 2021-10-15 3:05 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
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
arm64 allyesconfig
arm64 defconfig
arm allyesconfig
arm allmodconfig
i386 randconfig-c001-20211012
i386 randconfig-c001-20211011
i386 randconfig-c001-20211014
mips tb0219_defconfig
arm mvebu_v7_defconfig
xtensa audio_kc705_defconfig
ia64 zx1_defconfig
powerpc tqm8560_defconfig
sh se7705_defconfig
m68k alldefconfig
sh ul2_defconfig
sh hp6xx_defconfig
powerpc mpc837x_rdb_defconfig
mips sb1250_swarm_defconfig
sh defconfig
powerpc mpc83xx_defconfig
sh sh7710voipgw_defconfig
m68k hp300_defconfig
arm at91_dt_defconfig
mips ar7_defconfig
arm cns3420vb_defconfig
sh secureedge5410_defconfig
powerpc tqm8540_defconfig
powerpc motionpro_defconfig
arm corgi_defconfig
arm vexpress_defconfig
powerpc stx_gp3_defconfig
sh sh7785lcr_32bit_defconfig
powerpc obs600_defconfig
powerpc gamecube_defconfig
mips bmips_be_defconfig
sh se7619_defconfig
arm mps2_defconfig
h8300 allyesconfig
arm tegra_defconfig
mips lemote2f_defconfig
xtensa cadence_csp_defconfig
powerpc ps3_defconfig
powerpc xes_mpc85xx_defconfig
sh ecovec24-romimage_defconfig
sparc sparc64_defconfig
sh sh7757lcr_defconfig
powerpc microwatt_defconfig
mips ci20_defconfig
openrisc or1ksim_defconfig
powerpc akebono_defconfig
arm qcom_defconfig
sparc alldefconfig
powerpc warp_defconfig
sh kfr2r09-romimage_defconfig
arm u8500_defconfig
powerpc tqm8548_defconfig
um defconfig
mips maltaup_defconfig
parisc generic-32bit_defconfig
microblaze defconfig
sh sh7785lcr_defconfig
mips ip22_defconfig
powerpc mpc866_ads_defconfig
powerpc ge_imp3a_defconfig
xtensa generic_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
m68k m5272c3_defconfig
riscv nommu_k210_defconfig
xtensa xip_kc705_defconfig
powerpc linkstation_defconfig
arc haps_hs_defconfig
m68k m5249evb_defconfig
mips vocore2_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
arm multi_v7_defconfig
sh se7721_defconfig
arm gemini_defconfig
powerpc pasemi_defconfig
m68k apollo_defconfig
arm imx_v6_v7_defconfig
sh migor_defconfig
xtensa virt_defconfig
xtensa iss_defconfig
h8300 defconfig
m68k amiga_defconfig
powerpc mpc8560_ads_defconfig
arm footbridge_defconfig
m68k q40_defconfig
mips mpc30x_defconfig
powerpc ppc44x_defconfig
powerpc mpc832x_rdb_defconfig
powerpc tqm5200_defconfig
powerpc sequoia_defconfig
powerpc mpc8315_rdb_defconfig
mips bmips_stb_defconfig
powerpc eiger_defconfig
mips e55_defconfig
mips nlm_xlp_defconfig
powerpc tqm8xx_defconfig
powerpc chrp32_defconfig
arm spitz_defconfig
arc axs103_smp_defconfig
um x86_64_defconfig
powerpc klondike_defconfig
arm mmp2_defconfig
powerpc fsp2_defconfig
h8300 alldefconfig
arm socfpga_defconfig
arm pcm027_defconfig
mips qi_lb60_defconfig
arm omap1_defconfig
m68k sun3_defconfig
powerpc bluestone_defconfig
mips loongson2k_defconfig
mips loongson1b_defconfig
mips db1xxx_defconfig
arc nsimosci_defconfig
arm randconfig-c002-20211014
x86_64 randconfig-c001-20211014
arm randconfig-c002-20211011
x86_64 randconfig-c001-20211011
arm randconfig-c002-20211012
x86_64 randconfig-c001-20211012
ia64 allmodconfig
ia64 defconfig
ia64 allyesconfig
m68k allmodconfig
m68k defconfig
m68k allyesconfig
nios2 defconfig
nds32 allnoconfig
arc allyesconfig
nios2 allyesconfig
csky defconfig
alpha defconfig
alpha allyesconfig
arc defconfig
sh allmodconfig
xtensa allyesconfig
parisc defconfig
s390 defconfig
parisc allyesconfig
s390 allyesconfig
s390 allmodconfig
sparc allyesconfig
sparc defconfig
i386 defconfig
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
i386 randconfig-a001-20211012
i386 randconfig-a003-20211012
i386 randconfig-a004-20211012
i386 randconfig-a005-20211012
i386 randconfig-a002-20211012
i386 randconfig-a006-20211012
x86_64 randconfig-a015-20211011
x86_64 randconfig-a012-20211011
x86_64 randconfig-a016-20211011
x86_64 randconfig-a014-20211011
x86_64 randconfig-a013-20211011
x86_64 randconfig-a011-20211011
x86_64 randconfig-a004-20211012
x86_64 randconfig-a006-20211012
x86_64 randconfig-a001-20211012
x86_64 randconfig-a005-20211012
x86_64 randconfig-a002-20211012
x86_64 randconfig-a003-20211012
i386 randconfig-a016-20211011
i386 randconfig-a014-20211011
i386 randconfig-a011-20211011
i386 randconfig-a015-20211011
i386 randconfig-a012-20211011
i386 randconfig-a013-20211011
arc randconfig-r043-20211014
riscv nommu_virt_defconfig
riscv allnoconfig
riscv defconfig
riscv rv32_defconfig
riscv allyesconfig
riscv allmodconfig
x86_64 rhel-8.3-kselftests
um i386_defconfig
x86_64 defconfig
x86_64 rhel-8.3
x86_64 kexec
x86_64 allyesconfig
clang tested configs:
arm randconfig-c002-20211012
mips randconfig-c004-20211012
i386 randconfig-c001-20211012
s390 randconfig-c005-20211012
x86_64 randconfig-c007-20211012
powerpc randconfig-c003-20211012
riscv randconfig-c006-20211012
x86_64 randconfig-a004-20211011
x86_64 randconfig-a006-20211011
x86_64 randconfig-a001-20211011
x86_64 randconfig-a005-20211011
x86_64 randconfig-a002-20211011
x86_64 randconfig-a003-20211011
i386 randconfig-a001-20211011
i386 randconfig-a003-20211011
i386 randconfig-a004-20211011
i386 randconfig-a005-20211011
i386 randconfig-a002-20211011
i386 randconfig-a006-20211011
x86_64 randconfig-a015-20211012
x86_64 randconfig-a012-20211012
x86_64 randconfig-a016-20211012
x86_64 randconfig-a014-20211012
x86_64 randconfig-a013-20211012
x86_64 randconfig-a011-20211012
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-20211012
i386 randconfig-a014-20211012
i386 randconfig-a011-20211012
i386 randconfig-a015-20211012
i386 randconfig-a012-20211012
i386 randconfig-a013-20211012
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-20211012
s390 randconfig-r044-20211012
riscv randconfig-r042-20211012
hexagon randconfig-r045-20211012
hexagon randconfig-r041-20211011
hexagon randconfig-r045-20211011
hexagon randconfig-r041-20211014
s390 randconfig-r044-20211014
riscv randconfig-r042-20211014
hexagon randconfig-r045-20211014
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox