* Re: [PATCH 2/3] powerpc/book3e: Fix set_memory_x() and set_memory_nx()
From: Nicholas Piggin @ 2021-10-27 7:15 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Christophe Leroy, Michael Ellerman,
Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <8ccb9629-43fc-2f36-c9e4-61d6898fb80d@csgroup.eu>
Excerpts from Christophe Leroy's message of October 27, 2021 3:50 pm:
>
>
> Le 27/10/2021 à 07:27, Nicholas Piggin a écrit :
>> Excerpts from Christophe Leroy's message of October 27, 2021 2:55 pm:
>>>
>>>
>>> Le 27/10/2021 à 06:44, Nicholas Piggin a écrit :
>>>> Excerpts from Christophe Leroy's message of October 26, 2021 3:39 pm:
>>>>> set_memory_x() calls pte_mkexec() which sets _PAGE_EXEC.
>>>>> set_memory_nx() calls pte_exprotec() which clears _PAGE_EXEC.
>>>>>
>>>>> Book3e has 2 bits, UX and SX, which defines the exec rights
>>>>> resp. for user (PR=1) and for kernel (PR=0).
>>>>>
>>>>> _PAGE_EXEC is defined as UX only.
>>>>>
>>>>> An executable kernel page is set with either _PAGE_KERNEL_RWX
>>>>> or _PAGE_KERNEL_ROX, which both have SX set and UX cleared.
>>>>>
>>>>> So set_memory_nx() call for an executable kernel page does
>>>>> nothing because UX is already cleared.
>>>>>
>>>>> And set_memory_x() on a non-executable kernel page makes it
>>>>> executable for the user and keeps it non-executable for kernel.
>>>>>
>>>>> Also, pte_exec() always returns 'false' on kernel pages, because
>>>>> it checks _PAGE_EXEC which doesn't include SX, so for instance
>>>>> the W+X check doesn't work.
>>>>>
>>>>> To fix this:
>>>>> - change tlb_low_64e.S to use _PAGE_BAP_UX instead of _PAGE_USER
>>>>> - sets both UX and SX in _PAGE_EXEC so that pte_user() returns
>>>>> true whenever one of the two bits is set
>>>>
>>>> I don't understand this change. Which pte_user() returns true after
>>>> this change? Or do you mean pte_exec()?
>>>
>>> Oops, yes, I mean pte_exec()
>>>
>>> Unless I have to re-spin, can Michael eventually fix that typo while
>>> applying ?
>>>
>>>>
>>>> Does this filter through in some cases at least for kernel executable
>>>> PTEs will get both bits set? Seems cleaner to distinguish user and
>>>> kernel exec for that but maybe it's a lot of churn?
>>>
>>> Didn't understand what you mean.
>>>
>>> I did it like that to be able to continue using _PAGE_EXEC for checking
>>> executability regardless of whether this is user or kernel, and then
>>> continue using the generic nohash pte_exec() helper.
>>>
>>> Other solution would be to get rid of _PAGE_EXEC completely for book3e
>>> and implement both pte_exec() and pte_mkexec() with _PAGE_BAP_UX and
>>> _PAGE_BAP_SX, but I'm not sure it is worth the churn as you say. It
>>> would also mean different helpers for book3s/32 when it is using 32 bits
>>> PTE (CONFIG_PTE_64BIT=n)
>>
>> That's basically what I mean. And _PAGE_KERNEL_ROX etc would then not
>> set the UX bit. But at least for now it seems to be an improvement.
>>
>
> That's already the case:
>
> #define _PAGE_KERNEL_RWX (_PAGE_BAP_SW | _PAGE_BAP_SR | _PAGE_DIRTY |
> _PAGE_BAP_SX)
> #define _PAGE_KERNEL_ROX (_PAGE_BAP_SR | _PAGE_BAP_SX)
So it is, I was looking at the wrong header.
Looks okay to me then, for what it's worth.
Thanks,
Nick
^ permalink raw reply
* [PATCH] powerpc/security: Use a mutex for interrupt exit code patching
From: Russell Currey @ 2021-10-27 7:24 UTC (permalink / raw)
To: linuxppc-dev; +Cc: npiggin, Russell Currey
The mitigation-patching.sh script in the powerpc selftests toggles
all mitigations on and off simultaneously, revealing that rfi_flush
and stf_barrier cannot safely operate at the same time due to races
in updating the static key.
On some systems, the static key code throws a warning and the kernel
remains functional. On others, the kernel will hang or crash.
Fix this by slapping on a mutex.
Fixes: 13799748b957 ("powerpc/64: use interrupt restart table to speed up return from interrupt")
Signed-off-by: Russell Currey <ruscur@russell.cc>
---
arch/powerpc/lib/feature-fixups.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/arch/powerpc/lib/feature-fixups.c b/arch/powerpc/lib/feature-fixups.c
index cda17bee5afe..9956277fbd88 100644
--- a/arch/powerpc/lib/feature-fixups.c
+++ b/arch/powerpc/lib/feature-fixups.c
@@ -228,6 +228,7 @@ static void do_stf_exit_barrier_fixups(enum stf_barrier_type types)
static bool stf_exit_reentrant = false;
static bool rfi_exit_reentrant = false;
+static DEFINE_MUTEX(exit_flush_lock);
static int __do_stf_barrier_fixups(void *data)
{
@@ -253,6 +254,9 @@ void do_stf_barrier_fixups(enum stf_barrier_type types)
* low level interrupt exit code before patching. After the patching,
* if allowed, then flip the branch to allow fast exits.
*/
+
+ // Prevent static key update races with do_rfi_flush_fixups()
+ mutex_lock(&exit_flush_lock);
static_branch_enable(&interrupt_exit_not_reentrant);
stop_machine(__do_stf_barrier_fixups, &types, NULL);
@@ -264,6 +268,7 @@ void do_stf_barrier_fixups(enum stf_barrier_type types)
if (stf_exit_reentrant && rfi_exit_reentrant)
static_branch_disable(&interrupt_exit_not_reentrant);
+ mutex_unlock(&exit_flush_lock);
}
void do_uaccess_flush_fixups(enum l1d_flush_type types)
@@ -486,6 +491,9 @@ void do_rfi_flush_fixups(enum l1d_flush_type types)
* without stop_machine, so this could be achieved with a broadcast
* IPI instead, but this matches the stf sequence.
*/
+
+ // Prevent static key update races with do_stf_barrier_fixups()
+ mutex_lock(&exit_flush_lock);
static_branch_enable(&interrupt_exit_not_reentrant);
stop_machine(__do_rfi_flush_fixups, &types, NULL);
@@ -497,6 +505,7 @@ void do_rfi_flush_fixups(enum l1d_flush_type types)
if (stf_exit_reentrant && rfi_exit_reentrant)
static_branch_disable(&interrupt_exit_not_reentrant);
+ mutex_unlock(&exit_flush_lock);
}
void do_barrier_nospec_fixups_range(bool enable, void *fixup_start, void *fixup_end)
--
2.33.1
^ permalink raw reply related
* Re: instruction storage exception handling
From: Nicholas Piggin @ 2021-10-27 7:47 UTC (permalink / raw)
To: Christophe Leroy, Jacques de Laval, Scott Wood
Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <4ee635f5-7a67-bac5-2ad2-616c1aaa95b6@csgroup.eu>
Excerpts from Christophe Leroy's message of October 27, 2021 3:51 pm:
>
>
> Le 27/10/2021 à 07:25, Nicholas Piggin a écrit :
>> Excerpts from Christophe Leroy's message of October 27, 2021 3:00 pm:
>>>
>>>
>>> Le 27/10/2021 à 06:10, Nicholas Piggin a écrit :
>>>> Excerpts from Jacques de Laval's message of October 26, 2021 6:07 am:
>>>>> Hi,
>>>>>
>>>>> We are trying to upgrade kernel from 5.10 to 5.14.11. We have a Freescale/NXP
>>>>> T1023 SOC with two e5500 cores, and are running in 32-bit mode:
>>>>>
>>>>> CONFIG_PPC32=y
>>>>> # CONFIG_PPC64 is not set
>>>>>
>>>>> #
>>>>> # Processor support
>>>>> #
>>>>> # CONFIG_PPC_BOOK3S_32 is not set
>>>>> CONFIG_PPC_85xx=y
>>>>> # CONFIG_PPC_8xx is not set
>>>>> # CONFIG_40x is not set
>>>>> # CONFIG_44x is not set
>>>>> CONFIG_GENERIC_CPU=y
>>>>> # CONFIG_E5500_CPU is not set
>>>>> # CONFIG_E6500_CPU is not set
>>>>> CONFIG_E500=y
>>>>> CONFIG_PPC_E500MC=y
>>>>> CONFIG_PPC_FPU_REGS=y
>>>>> CONFIG_PPC_FPU=y
>>>>> CONFIG_FSL_EMB_PERFMON=y
>>>>> CONFIG_FSL_EMB_PERF_EVENT=y
>>>>> CONFIG_FSL_EMB_PERF_EVENT_E500=y
>>>>> CONFIG_BOOKE=y
>>>>> CONFIG_FSL_BOOKE=y
>>>>> CONFIG_PPC_FSL_BOOK3E=y
>>>>> CONFIG_PTE_64BIT=y
>>>>> CONFIG_PHYS_64BIT=y
>>>>> CONFIG_PPC_MMU_NOHASH=y
>>>>> CONFIG_PPC_BOOK3E_MMU=y
>>>>> # CONFIG_PMU_SYSFS is not set
>>>>> CONFIG_SMP=y
>>>>> CONFIG_NR_CPUS=2
>>>>> CONFIG_PPC_DOORBELL=y
>>>>> # end of Processor support
>>>>>
>>>>> We compile using 32-bit Bootlin PPC toolchain:
>>>>>
>>>>> powerpc-e500mc glibc bleeding-edge 2020.08-1.
>>>>>
>>>>> When booting, and starting PID 1 we sometimes get a hang. Nothing but our init
>>>>> process is running, and for debugging purposes our init currently looks like
>>>>> this:
>>>>>
>>>>> int main(int argc, char *argv[]){
>>>>> for (int i = 0; ; i++) {
>>>>> FILE *fp = fopen("/dev/kmsg", "w");
>>>>> if (fp) {
>>>>> fprintf(fp, "%d\n", i);
>>>>> fclose(fp);
>>>>> }
>>>>> sleep(1);
>>>>> }
>>>>> return 0;
>>>>> }
>>>>>
>>>>> When the hangup occur we don't get any output at all from our PID 1.
>>>>> The last output is from the kernel:
>>>>>
>>>>> Run /sbin/init as init process
>>>>> with arguments:
>>>>> /sbin/init
>>>>> with environment:
>>>>> HOME=/
>>>>> TERM=linux
>>>>> kgdboc=ttyS0,115200
>>>>>
>>>>> When issuing a backtrace on all active cpus we can see that the kernel is
>>>>> handling an instruction storage exception:
>>>>>
>>>>> sysrq: Show backtrace of all active CPUs
>>>>> sysrq: CPU0:
>>>>> CPU: 0 PID: 1 Comm: init Not tainted 5.14.11 #1
>>>>> NIP: c02aac78 LR: c02aac2c CTR: 00000000
>>>>> REGS: c1907d40 TRAP: 0500 Not tainted (5.14.11)
>>>>> MSR: 00029002 <CE,EE,ME> CR: 82244284 XER: 20000000
>>>>> GPR00: 0000000f c1907e20 c1910000 0065a000 000001d0 01100cca c1907e84 0000000c
>>>>> GPR08: d39a8000 000001d3 0000000c c1907f10 42244284 00000000 00740514 bfb71670
>>>>> GPR16: 007040e6 00701418 b7c1a5f0 00702f18 00000000 bfb71690 0000fff1 b7c1c478
>>>>> GPR24: 00708558 00701698 d3994040 00029002 c1907f20 0065a238 00000355 d39a2790
>>>>> NIP [c02aac78] handle_mm_fault+0xf8/0x11f0
>>>>> LR [c02aac2c] handle_mm_fault+0xac/0x11f0
>>>>> Call Trace:
>>>>> [c1907e20] [c02aac10] handle_mm_fault+0x90/0x11f0 (unreliable)
>>>>> [c1907ec0] [c003078c] ___do_page_fault+0x26c/0x780
>>>>> [c1907ef0] [c0030cd4] do_page_fault+0x34/0x100
>>>>> [c1907f10] [c0000988] InstructionStorage+0x108/0x120
>>>>> --- interrupt: 400 at 0x65a238
>>>>> NIP: 0065a238 LR: 0052f26c CTR: 0052f260
>>>>> REGS: c1907f20 TRAP: 0400 Not tainted (5.14.11)
>>>>> MSR: 0002d002 <CE,EE,PR,ME> CR: 42242284 XER: 00000000
>>>>> GPR00: b7be9914 bfb71620 b7c203a0 8c008000 0070400d b7c182a0 000b8260 0052f260
>>>>> GPR08: 0047d448 0052f260 0000000a 00000003 42242284 00000000 00740514 bfb71670
>>>>> GPR16: 007040e6 00701418 b7c1a5f0 00702f18 00000000 bfb71690 0000fff1 b7c1c478
>>>>> GPR24: 00708558 00701698 00700000 00000015 b7c1c2b0 00707e20 b7c1b8a8 bfb71660
>>>>> NIP [0065a238] 0x65a238
>>>>> LR [0052f26c] 0x52f26c
>>>>> --- interrupt: 400
>>>>> Instruction dump:
>>>>> 60a500c0 811f0020 57aa6cfa 813f0000 57a30026 809f004c 81080024 7d29e850
>>>>> 90a1002c 5529a33e 93c10038 7d244a14 <90610034> 7d485215 91210030 41c203dc
>>>>>
>>>>> We have also observed that the CPU is continuously servicing the same interrupt
>>>>> (north of 140k times per sec), it is not deadlocked.
>>>>>
>>>>> We have not yet been able to reproduce this behavior under QEMU system
>>>>> emulation.
>>>>>
>>>>> When bisecting between 5.10 and 5.14.11 we can see that this behavior started
>>>>> with commit a01a3f2ddbcda83e8572787c0ec1dcbeba86915a:
>>>>>
>>>>> powerpc: remove arguments from fault handler functions
>>>>
>>>> Thank you for the excellent work to investigate and report this.
>>>>
>>>>>
>>>>> Our best guess that the instruction storage exception is not properly handled
>>>>> and the kernel is never able to recover from the page fault, but we don't
>>>>> really know how to proceed. Does anyone have any suggestions or insights?
>>>>
>>>> Before my patch, zero was passed to the page fault error code, after
>>>> my patch it passes the contents of ESR SPR.
>>>>
>>>> It looks like the BookE instruction access interrupt does not set ESR
>>>> (except for BO interrupts maybe?) so you're getting what was in the ESR
>>>> register from a previous interrupt, and maybe if that was a store then
>>>> access_error won't cause a segfault because is_exec is true so that
>>>> test bails out early, then it might just keep retrying the interrupt.
>>>>
>>>> That could explain why you don't always see the same thing.
>>>>
>>>> Now previous code still saved ESR in regs->esr/dsisr for some reason.
>>>> I can't quite see why that should have been necessary though. Does
>
> According to the e500 Reference Manual, on ISI:
>
> BO is set if the instruction fetch caused a byte-ordering exception;
> otherwise cleared. *All* other defined ESR bits are *cleared*.
You're right. In that case it shouldn't change anything unless there
was a BO fault. I'm not sure what the problem is then. Guessing based
on the NIP and instructions, it looks like it's probably got the correct
user address that it's storing into vmf on the stack, so it has got past
the access checks so my theory would be wrong anyway.
Must be something simple but I can't see it yet.
Thanks,
Nick
^ permalink raw reply
* Re: instruction storage exception handling
From: Christophe Leroy @ 2021-10-27 7:52 UTC (permalink / raw)
To: Nicholas Piggin, Jacques de Laval, Scott Wood
Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1635318932.od1ierwsis.astroid@bobo.none>
Le 27/10/2021 à 09:47, Nicholas Piggin a écrit :
> Excerpts from Christophe Leroy's message of October 27, 2021 3:51 pm:
>>
>>
>> Le 27/10/2021 à 07:25, Nicholas Piggin a écrit :
>>> Excerpts from Christophe Leroy's message of October 27, 2021 3:00 pm:
>>>>
>>>>
>>>> Le 27/10/2021 à 06:10, Nicholas Piggin a écrit :
>>>>> Excerpts from Jacques de Laval's message of October 26, 2021 6:07 am:
>>>>>> Hi,
>>>>>>
>>>>>> We are trying to upgrade kernel from 5.10 to 5.14.11. We have a Freescale/NXP
>>>>>> T1023 SOC with two e5500 cores, and are running in 32-bit mode:
>>>>>>
>>>>>> CONFIG_PPC32=y
>>>>>> # CONFIG_PPC64 is not set
>>>>>>
>>>>>> #
>>>>>> # Processor support
>>>>>> #
>>>>>> # CONFIG_PPC_BOOK3S_32 is not set
>>>>>> CONFIG_PPC_85xx=y
>>>>>> # CONFIG_PPC_8xx is not set
>>>>>> # CONFIG_40x is not set
>>>>>> # CONFIG_44x is not set
>>>>>> CONFIG_GENERIC_CPU=y
>>>>>> # CONFIG_E5500_CPU is not set
>>>>>> # CONFIG_E6500_CPU is not set
>>>>>> CONFIG_E500=y
>>>>>> CONFIG_PPC_E500MC=y
>>>>>> CONFIG_PPC_FPU_REGS=y
>>>>>> CONFIG_PPC_FPU=y
>>>>>> CONFIG_FSL_EMB_PERFMON=y
>>>>>> CONFIG_FSL_EMB_PERF_EVENT=y
>>>>>> CONFIG_FSL_EMB_PERF_EVENT_E500=y
>>>>>> CONFIG_BOOKE=y
>>>>>> CONFIG_FSL_BOOKE=y
>>>>>> CONFIG_PPC_FSL_BOOK3E=y
>>>>>> CONFIG_PTE_64BIT=y
>>>>>> CONFIG_PHYS_64BIT=y
>>>>>> CONFIG_PPC_MMU_NOHASH=y
>>>>>> CONFIG_PPC_BOOK3E_MMU=y
>>>>>> # CONFIG_PMU_SYSFS is not set
>>>>>> CONFIG_SMP=y
>>>>>> CONFIG_NR_CPUS=2
>>>>>> CONFIG_PPC_DOORBELL=y
>>>>>> # end of Processor support
>>>>>>
>>>>>> We compile using 32-bit Bootlin PPC toolchain:
>>>>>>
>>>>>> powerpc-e500mc glibc bleeding-edge 2020.08-1.
>>>>>>
>>>>>> When booting, and starting PID 1 we sometimes get a hang. Nothing but our init
>>>>>> process is running, and for debugging purposes our init currently looks like
>>>>>> this:
>>>>>>
>>>>>> int main(int argc, char *argv[]){
>>>>>> for (int i = 0; ; i++) {
>>>>>> FILE *fp = fopen("/dev/kmsg", "w");
>>>>>> if (fp) {
>>>>>> fprintf(fp, "%d\n", i);
>>>>>> fclose(fp);
>>>>>> }
>>>>>> sleep(1);
>>>>>> }
>>>>>> return 0;
>>>>>> }
>>>>>>
>>>>>> When the hangup occur we don't get any output at all from our PID 1.
>>>>>> The last output is from the kernel:
>>>>>>
>>>>>> Run /sbin/init as init process
>>>>>> with arguments:
>>>>>> /sbin/init
>>>>>> with environment:
>>>>>> HOME=/
>>>>>> TERM=linux
>>>>>> kgdboc=ttyS0,115200
>>>>>>
>>>>>> When issuing a backtrace on all active cpus we can see that the kernel is
>>>>>> handling an instruction storage exception:
>>>>>>
>>>>>> sysrq: Show backtrace of all active CPUs
>>>>>> sysrq: CPU0:
>>>>>> CPU: 0 PID: 1 Comm: init Not tainted 5.14.11 #1
>>>>>> NIP: c02aac78 LR: c02aac2c CTR: 00000000
>>>>>> REGS: c1907d40 TRAP: 0500 Not tainted (5.14.11)
>>>>>> MSR: 00029002 <CE,EE,ME> CR: 82244284 XER: 20000000
>>>>>> GPR00: 0000000f c1907e20 c1910000 0065a000 000001d0 01100cca c1907e84 0000000c
>>>>>> GPR08: d39a8000 000001d3 0000000c c1907f10 42244284 00000000 00740514 bfb71670
>>>>>> GPR16: 007040e6 00701418 b7c1a5f0 00702f18 00000000 bfb71690 0000fff1 b7c1c478
>>>>>> GPR24: 00708558 00701698 d3994040 00029002 c1907f20 0065a238 00000355 d39a2790
>>>>>> NIP [c02aac78] handle_mm_fault+0xf8/0x11f0
>>>>>> LR [c02aac2c] handle_mm_fault+0xac/0x11f0
>>>>>> Call Trace:
>>>>>> [c1907e20] [c02aac10] handle_mm_fault+0x90/0x11f0 (unreliable)
>>>>>> [c1907ec0] [c003078c] ___do_page_fault+0x26c/0x780
>>>>>> [c1907ef0] [c0030cd4] do_page_fault+0x34/0x100
>>>>>> [c1907f10] [c0000988] InstructionStorage+0x108/0x120
>>>>>> --- interrupt: 400 at 0x65a238
>>>>>> NIP: 0065a238 LR: 0052f26c CTR: 0052f260
>>>>>> REGS: c1907f20 TRAP: 0400 Not tainted (5.14.11)
>>>>>> MSR: 0002d002 <CE,EE,PR,ME> CR: 42242284 XER: 00000000
>>>>>> GPR00: b7be9914 bfb71620 b7c203a0 8c008000 0070400d b7c182a0 000b8260 0052f260
>>>>>> GPR08: 0047d448 0052f260 0000000a 00000003 42242284 00000000 00740514 bfb71670
>>>>>> GPR16: 007040e6 00701418 b7c1a5f0 00702f18 00000000 bfb71690 0000fff1 b7c1c478
>>>>>> GPR24: 00708558 00701698 00700000 00000015 b7c1c2b0 00707e20 b7c1b8a8 bfb71660
>>>>>> NIP [0065a238] 0x65a238
>>>>>> LR [0052f26c] 0x52f26c
>>>>>> --- interrupt: 400
>>>>>> Instruction dump:
>>>>>> 60a500c0 811f0020 57aa6cfa 813f0000 57a30026 809f004c 81080024 7d29e850
>>>>>> 90a1002c 5529a33e 93c10038 7d244a14 <90610034> 7d485215 91210030 41c203dc
>>>>>>
>>>>>> We have also observed that the CPU is continuously servicing the same interrupt
>>>>>> (north of 140k times per sec), it is not deadlocked.
>>>>>>
>>>>>> We have not yet been able to reproduce this behavior under QEMU system
>>>>>> emulation.
>>>>>>
>>>>>> When bisecting between 5.10 and 5.14.11 we can see that this behavior started
>>>>>> with commit a01a3f2ddbcda83e8572787c0ec1dcbeba86915a:
>>>>>>
>>>>>> powerpc: remove arguments from fault handler functions
>>>>>
>>>>> Thank you for the excellent work to investigate and report this.
>>>>>
>>>>>>
>>>>>> Our best guess that the instruction storage exception is not properly handled
>>>>>> and the kernel is never able to recover from the page fault, but we don't
>>>>>> really know how to proceed. Does anyone have any suggestions or insights?
>>>>>
>>>>> Before my patch, zero was passed to the page fault error code, after
>>>>> my patch it passes the contents of ESR SPR.
>>>>>
>>>>> It looks like the BookE instruction access interrupt does not set ESR
>>>>> (except for BO interrupts maybe?) so you're getting what was in the ESR
>>>>> register from a previous interrupt, and maybe if that was a store then
>>>>> access_error won't cause a segfault because is_exec is true so that
>>>>> test bails out early, then it might just keep retrying the interrupt.
>>>>>
>>>>> That could explain why you don't always see the same thing.
>>>>>
>>>>> Now previous code still saved ESR in regs->esr/dsisr for some reason.
>>>>> I can't quite see why that should have been necessary though. Does
>>
>> According to the e500 Reference Manual, on ISI:
>>
>> BO is set if the instruction fetch caused a byte-ordering exception;
>> otherwise cleared. *All* other defined ESR bits are *cleared*.
>
> You're right. In that case it shouldn't change anything unless there
> was a BO fault. I'm not sure what the problem is then. Guessing based
> on the NIP and instructions, it looks like it's probably got the correct
> user address that it's storing into vmf on the stack, so it has got past
> the access checks so my theory would be wrong anyway.
>
> Must be something simple but I can't see it yet.
>
Anyway, I think it is still worth doing the check with setting 0 in
_ESR(r11), maybe the Reference Manual is wrong.
So Jacques, please do the test anyway if you can.
Thanks
Christophe
^ permalink raw reply
* Re: [PATCH 1/2] powerpc/watchdog: prevent printk and send IPI while holding the wd lock
From: Laurent Dufour @ 2021-10-27 8:14 UTC (permalink / raw)
To: Nicholas Piggin, benh, mpe, paulus; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1635303699.wgz87uxy4c.astroid@bobo.none>
Le 27/10/2021 à 05:29, Nicholas Piggin a écrit :
> Excerpts from Laurent Dufour's message of October 27, 2021 2:27 am:
>> When handling the Watchdog interrupt, long processing should not be done
>> while holding the __wd_smp_lock. This prevents the other CPUs to grab it
>> and to process Watchdog timer interrupts. Furhtermore, this could lead to
>> the following situation:
>>
>> CPU x detect lockup on CPU y and grab the __wd_smp_lock
>> in watchdog_smp_panic()
>> CPU y caught the watchdog interrupt and try to grab the __wd_smp_lock
>> in soft_nmi_interrupt()
>> CPU x wait for CPU y to catch the IPI for 1s in __smp_send_nmi_ipi()
>
> CPU y should get the IPI here if it's a NMI IPI (which will be true for
>> = POWER9 64s).
>
> That said, not all platforms support it and the console lock problem
> seems real, so okay.
>
>> CPU x will timeout and so has spent 1s waiting while holding the
>> __wd_smp_lock.
>>
>> A deadlock may also happen between the __wd_smp_lock and the console_owner
>> 'lock' this way:
>> CPU x grab the console_owner
>> CPU y grab the __wd_smp_lock
>> CPU x catch the watchdog timer interrupt and needs to grab __wd_smp_lock
>> CPU y wants to print something and wait for console_owner
>> -> deadlock
>>
>> Doing all the long processing without holding the _wd_smp_lock prevents
>> these situations.
>
> The intention was to avoid logs getting garbled e.g., if multiple
> different CPUs fire at once.
>
> I wonder if instead we could deal with that by protecting the IPI
> sending and printing stuff with a trylock, and if you don't get the
> trylock then just return, and you'll come back with the next timer
> interrupt.
That sounds a bit risky to me, especially on large system when system goes
wrong, all the CPU may try lock here.
Furthermore, now operation done under the lock protection are quite fast, there
is no more spinning like the delay loop done when sending an IPI.
Protecting the IPI sending is a nightmare, if the target CPU is later play with
the lock we are taking during the IPI processing, furthermore, if the target CPU
is not responding the sending CPU is waiting for 1s, which slows all the system
due to the lock held.
Since I do a copy of the pending CPU mask and clear it under the lock
protection, the IPI sending is safe while done without holding the lock.
Regarding the interleaved traces, I don't think this has to be managed down
here, but rather in the printk/console path.
Cheers,
Laurent.
>
> Thanks,
> Nick
>
>>
>> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
>> ---
>> arch/powerpc/kernel/watchdog.c | 31 +++++++++++++++++--------------
>> 1 file changed, 17 insertions(+), 14 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/watchdog.c b/arch/powerpc/kernel/watchdog.c
>> index f9ea0e5357f9..bc7411327066 100644
>> --- a/arch/powerpc/kernel/watchdog.c
>> +++ b/arch/powerpc/kernel/watchdog.c
>> @@ -149,6 +149,8 @@ static void set_cpu_stuck(int cpu, u64 tb)
>>
>> static void watchdog_smp_panic(int cpu, u64 tb)
>> {
>> + cpumask_t cpus_pending_copy;
>> + u64 last_reset_tb_copy;
>> unsigned long flags;
>> int c;
>>
>> @@ -161,29 +163,32 @@ static void watchdog_smp_panic(int cpu, u64 tb)
>> if (cpumask_weight(&wd_smp_cpus_pending) == 0)
>> goto out;
>>
>> + cpumask_copy(&cpus_pending_copy, &wd_smp_cpus_pending);
>> + last_reset_tb_copy = wd_smp_last_reset_tb;
>> +
>> + /* Take the stuck CPUs out of the watch group */
>> + set_cpumask_stuck(&wd_smp_cpus_pending, tb);
>> +
>> + wd_smp_unlock(&flags);
>> +
>> pr_emerg("CPU %d detected hard LOCKUP on other CPUs %*pbl\n",
>> - cpu, cpumask_pr_args(&wd_smp_cpus_pending));
>> + cpu, cpumask_pr_args(&cpus_pending_copy));
>> pr_emerg("CPU %d TB:%lld, last SMP heartbeat TB:%lld (%lldms ago)\n",
>> - cpu, tb, wd_smp_last_reset_tb,
>> - tb_to_ns(tb - wd_smp_last_reset_tb) / 1000000);
>> + cpu, tb, last_reset_tb_copy,
>> + tb_to_ns(tb - last_reset_tb_copy) / 1000000);
>>
>> if (!sysctl_hardlockup_all_cpu_backtrace) {
>> /*
>> * Try to trigger the stuck CPUs, unless we are going to
>> * get a backtrace on all of them anyway.
>> */
>> - for_each_cpu(c, &wd_smp_cpus_pending) {
>> + for_each_cpu(c, &cpus_pending_copy) {
>> if (c == cpu)
>> continue;
>> smp_send_nmi_ipi(c, wd_lockup_ipi, 1000000);
>> }
>> }
>>
>> - /* Take the stuck CPUs out of the watch group */
>> - set_cpumask_stuck(&wd_smp_cpus_pending, tb);
>> -
>> - wd_smp_unlock(&flags);
>> -
>> if (sysctl_hardlockup_all_cpu_backtrace)
>> trigger_allbutself_cpu_backtrace();
>>
>> @@ -204,6 +209,8 @@ static void wd_smp_clear_cpu_pending(int cpu, u64 tb)
>> unsigned long flags;
>>
>> wd_smp_lock(&flags);
>> + cpumask_clear_cpu(cpu, &wd_smp_cpus_stuck);
>> + wd_smp_unlock(&flags);
>>
>> pr_emerg("CPU %d became unstuck TB:%lld\n",
>> cpu, tb);
>> @@ -212,9 +219,6 @@ static void wd_smp_clear_cpu_pending(int cpu, u64 tb)
>> show_regs(regs);
>> else
>> dump_stack();
>> -
>> - cpumask_clear_cpu(cpu, &wd_smp_cpus_stuck);
>> - wd_smp_unlock(&flags);
>> }
>> return;
>> }
>> @@ -267,6 +271,7 @@ DEFINE_INTERRUPT_HANDLER_NMI(soft_nmi_interrupt)
>> return 0;
>> }
>> set_cpu_stuck(cpu, tb);
>> + wd_smp_unlock(&flags);
>>
>> pr_emerg("CPU %d self-detected hard LOCKUP @ %pS\n",
>> cpu, (void *)regs->nip);
>> @@ -277,8 +282,6 @@ DEFINE_INTERRUPT_HANDLER_NMI(soft_nmi_interrupt)
>> print_irqtrace_events(current);
>> show_regs(regs);
>>
>> - wd_smp_unlock(&flags);
>> -
>> if (sysctl_hardlockup_all_cpu_backtrace)
>> trigger_allbutself_cpu_backtrace();
>>
>> --
>> 2.33.1
>>
>>
^ permalink raw reply
* Re: [PATCH 1/2] powerpc/watchdog: prevent printk and send IPI while holding the wd lock
From: Nicholas Piggin @ 2021-10-27 8:51 UTC (permalink / raw)
To: benh, Laurent Dufour, mpe, paulus; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <33e15005-d342-5270-9b9d-64750f8794a7@linux.ibm.com>
Excerpts from Laurent Dufour's message of October 27, 2021 6:14 pm:
> Le 27/10/2021 à 05:29, Nicholas Piggin a écrit :
>> Excerpts from Laurent Dufour's message of October 27, 2021 2:27 am:
>>> When handling the Watchdog interrupt, long processing should not be done
>>> while holding the __wd_smp_lock. This prevents the other CPUs to grab it
>>> and to process Watchdog timer interrupts. Furhtermore, this could lead to
>>> the following situation:
>>>
>>> CPU x detect lockup on CPU y and grab the __wd_smp_lock
>>> in watchdog_smp_panic()
>>> CPU y caught the watchdog interrupt and try to grab the __wd_smp_lock
>>> in soft_nmi_interrupt()
>>> CPU x wait for CPU y to catch the IPI for 1s in __smp_send_nmi_ipi()
>>
>> CPU y should get the IPI here if it's a NMI IPI (which will be true for
>>> = POWER9 64s).
>>
>> That said, not all platforms support it and the console lock problem
>> seems real, so okay.
>>
>>> CPU x will timeout and so has spent 1s waiting while holding the
>>> __wd_smp_lock.
>>>
>>> A deadlock may also happen between the __wd_smp_lock and the console_owner
>>> 'lock' this way:
>>> CPU x grab the console_owner
>>> CPU y grab the __wd_smp_lock
>>> CPU x catch the watchdog timer interrupt and needs to grab __wd_smp_lock
>>> CPU y wants to print something and wait for console_owner
>>> -> deadlock
>>>
>>> Doing all the long processing without holding the _wd_smp_lock prevents
>>> these situations.
>>
>> The intention was to avoid logs getting garbled e.g., if multiple
>> different CPUs fire at once.
>>
>> I wonder if instead we could deal with that by protecting the IPI
>> sending and printing stuff with a trylock, and if you don't get the
>> trylock then just return, and you'll come back with the next timer
>> interrupt.
>
> That sounds a bit risky to me, especially on large system when system goes
> wrong, all the CPU may try lock here.
That should be okay though, one will get through and the others will
skip.
> Furthermore, now operation done under the lock protection are quite fast, there
> is no more spinning like the delay loop done when sending an IPI.
>
> Protecting the IPI sending is a nightmare, if the target CPU is later play with
> the lock we are taking during the IPI processing, furthermore, if the target CPU
> is not responding the sending CPU is waiting for 1s, which slows all the system
> due to the lock held.
> Since I do a copy of the pending CPU mask and clear it under the lock
> protection, the IPI sending is safe while done without holding the lock.
Protecting IPI sending basically has all the same issues in the NMI
IPI layer.
>
> Regarding the interleaved traces, I don't think this has to be managed down
> here, but rather in the printk/console path.
It can't necessarily be because some of the problem is actually that a
NMI handler can be interrupted by another NMI IPI because the caller
can return only after handlers start running rather than complete.
I don't think it would be an additional nightmare to trylock.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH 1/2] powerpc/watchdog: prevent printk and send IPI while holding the wd lock
From: Nicholas Piggin @ 2021-10-27 9:49 UTC (permalink / raw)
To: benh, Laurent Dufour, mpe, paulus; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1635303699.wgz87uxy4c.astroid@bobo.none>
Excerpts from Nicholas Piggin's message of October 27, 2021 1:29 pm:
> Excerpts from Laurent Dufour's message of October 27, 2021 2:27 am:
>> When handling the Watchdog interrupt, long processing should not be done
>> while holding the __wd_smp_lock. This prevents the other CPUs to grab it
>> and to process Watchdog timer interrupts. Furhtermore, this could lead to
>> the following situation:
>>
>> CPU x detect lockup on CPU y and grab the __wd_smp_lock
>> in watchdog_smp_panic()
>> CPU y caught the watchdog interrupt and try to grab the __wd_smp_lock
>> in soft_nmi_interrupt()
>> CPU x wait for CPU y to catch the IPI for 1s in __smp_send_nmi_ipi()
>
> CPU y should get the IPI here if it's a NMI IPI (which will be true for
>>= POWER9 64s).
>
> That said, not all platforms support it and the console lock problem
> seems real, so okay.
>
>> CPU x will timeout and so has spent 1s waiting while holding the
>> __wd_smp_lock.
>>
>> A deadlock may also happen between the __wd_smp_lock and the console_owner
>> 'lock' this way:
>> CPU x grab the console_owner
>> CPU y grab the __wd_smp_lock
>> CPU x catch the watchdog timer interrupt and needs to grab __wd_smp_lock
>> CPU y wants to print something and wait for console_owner
>> -> deadlock
>>
>> Doing all the long processing without holding the _wd_smp_lock prevents
>> these situations.
>
> The intention was to avoid logs getting garbled e.g., if multiple
> different CPUs fire at once.
>
> I wonder if instead we could deal with that by protecting the IPI
> sending and printing stuff with a trylock, and if you don't get the
> trylock then just return, and you'll come back with the next timer
> interrupt.
Something like this (untested) should basically hold off concurrency on
watchdog panics. It does not serialize output from IPI targets but it
should prevent multiple CPUs trying to send IPIs at once, without
holding the lock.
---
diff --git a/arch/powerpc/kernel/watchdog.c b/arch/powerpc/kernel/watchdog.c
index 2ffeb3f8b5a7..3a0db577da56 100644
--- a/arch/powerpc/kernel/watchdog.c
+++ b/arch/powerpc/kernel/watchdog.c
@@ -85,6 +85,7 @@ static DEFINE_PER_CPU(u64, wd_timer_tb);
/* SMP checker bits */
static unsigned long __wd_smp_lock;
+static unsigned long __wd_printing;
static cpumask_t wd_smp_cpus_pending;
static cpumask_t wd_smp_cpus_stuck;
static u64 wd_smp_last_reset_tb;
@@ -131,10 +132,10 @@ static void wd_lockup_ipi(struct pt_regs *regs)
/* Do not panic from here because that can recurse into NMI IPI layer */
}
-static void set_cpumask_stuck(const struct cpumask *cpumask, u64 tb)
+static void set_cpu_stuck(int cpu, u64 tb)
{
- cpumask_or(&wd_smp_cpus_stuck, &wd_smp_cpus_stuck, cpumask);
- cpumask_andnot(&wd_smp_cpus_pending, &wd_smp_cpus_pending, cpumask);
+ cpumask_set_cpu(cpu, &wd_smp_cpus_stuck);
+ cpumask_clear_cpu(cpu, &wd_smp_cpus_pending);
if (cpumask_empty(&wd_smp_cpus_pending)) {
wd_smp_last_reset_tb = tb;
cpumask_andnot(&wd_smp_cpus_pending,
@@ -142,10 +143,6 @@ static void set_cpumask_stuck(const struct cpumask *cpumask, u64 tb)
&wd_smp_cpus_stuck);
}
}
-static void set_cpu_stuck(int cpu, u64 tb)
-{
- set_cpumask_stuck(cpumask_of(cpu), tb);
-}
static void watchdog_smp_panic(int cpu, u64 tb)
{
@@ -160,6 +157,10 @@ static void watchdog_smp_panic(int cpu, u64 tb)
goto out;
if (cpumask_weight(&wd_smp_cpus_pending) == 0)
goto out;
+ if (__wd_printing)
+ goto out;
+ __wd_printing = 1;
+ wd_smp_unlock(&flags);
pr_emerg("CPU %d detected hard LOCKUP on other CPUs %*pbl\n",
cpu, cpumask_pr_args(&wd_smp_cpus_pending));
@@ -172,24 +173,31 @@ static void watchdog_smp_panic(int cpu, u64 tb)
* Try to trigger the stuck CPUs, unless we are going to
* get a backtrace on all of them anyway.
*/
- for_each_cpu(c, &wd_smp_cpus_pending) {
+ for_each_online_cpu(c) {
if (c == cpu)
continue;
+ if (!cpumask_test_cpu(cpu, &wd_smp_cpus_pending))
+ continue;
+ wd_smp_lock(&flags);
+ if (!cpumask_test_cpu(cpu, &wd_smp_cpus_pending)) {
+ wd_smp_unlock(&flags);
+ continue;
+ }
+ /* Take the stuck CPU out of the watch group */
+ set_cpu_stuck(cpu, tb);
+ wd_smp_unlock(&flags);
+
smp_send_nmi_ipi(c, wd_lockup_ipi, 1000000);
}
}
- /* Take the stuck CPUs out of the watch group */
- set_cpumask_stuck(&wd_smp_cpus_pending, tb);
-
- wd_smp_unlock(&flags);
-
if (sysctl_hardlockup_all_cpu_backtrace)
trigger_allbutself_cpu_backtrace();
if (hardlockup_panic)
nmi_panic(NULL, "Hard LOCKUP");
+ __wd_printing = 0;
return;
out:
@@ -204,8 +212,6 @@ static void wd_smp_clear_cpu_pending(int cpu, u64 tb)
if (unlikely(cpumask_test_cpu(cpu, &wd_smp_cpus_stuck))) {
struct pt_regs *regs = get_irq_regs();
- wd_smp_lock(&flags);
-
pr_emerg("CPU %d became unstuck TB:%lld\n",
cpu, tb);
print_irqtrace_events(current);
@@ -214,6 +220,7 @@ static void wd_smp_clear_cpu_pending(int cpu, u64 tb)
else
dump_stack();
+ wd_smp_lock(&flags);
cpumask_clear_cpu(cpu, &wd_smp_cpus_stuck);
wd_smp_unlock(&flags);
}
@@ -263,8 +270,16 @@ DEFINE_INTERRUPT_HANDLER_NMI(soft_nmi_interrupt)
wd_smp_unlock(&flags);
return 0;
}
+ if (__wd_printing) {
+ wd_smp_unlock(&flags);
+ return 0;
+ }
+ __wd_printing = 1;
+
set_cpu_stuck(cpu, tb);
+ wd_smp_unlock(&flags);
+
pr_emerg("CPU %d self-detected hard LOCKUP @ %pS\n",
cpu, (void *)regs->nip);
pr_emerg("CPU %d TB:%lld, last heartbeat TB:%lld (%lldms ago)\n",
@@ -274,13 +289,13 @@ DEFINE_INTERRUPT_HANDLER_NMI(soft_nmi_interrupt)
print_irqtrace_events(current);
show_regs(regs);
- wd_smp_unlock(&flags);
-
if (sysctl_hardlockup_all_cpu_backtrace)
trigger_allbutself_cpu_backtrace();
if (hardlockup_panic)
nmi_panic(regs, "Hard LOCKUP");
+
+ __wd_printing = 0;
}
if (wd_panic_timeout_tb < 0x7fffffff)
mtspr(SPRN_DEC, wd_panic_timeout_tb);
^ permalink raw reply related
* Re: Linux kernel: powerpc: KVM guest can trigger host crash on Power8
From: John Paul Adrian Glaubitz @ 2021-10-27 10:03 UTC (permalink / raw)
To: Michael Ellerman
Cc: oss-security, debian-powerpc@lists.debian.org, linuxppc-dev
In-Reply-To: <878ryfavaz.fsf@mpe.ellerman.id.au>
Hi Michael!
On 10/27/21 07:30, Michael Ellerman wrote:
> I did test the repro case you gave me before (in the bugzilla), which
> was building glibc, that passes for me with a patched host.
Did you manage to crash the unpatched host? If the unpatched host crashes
for you but the patched doesn't, I will make sure I didn't accidentally
miss anything.
Also, I'll try a kernel from git with Debian's config.
> I guess we have yet another bug.
>
> I tried the following in a debian BE VM and it completed fine:
>
> $ dget -u http://ftp.debian.org/debian/pool/main/g/git/git_2.33.1-1.dsc
> $ sbuild -d sid --arch=powerpc --no-arch-all git_2.33.1-1.dsc
>
> Same for ppc64.
>
> And I also tried both at once, repeatedly in a loop.
Did you try building gcc-11 for powerpc and ppc64 both at once?
> I guess it's something more complicated.
>
> What exact host/guest kernel versions and configs are you running?
Both the host and guest are running Debian's stock 5.14.12 kernel. The host has
a kernel with your patches applied, the guest doesn't.
Let me do some more testing.
Adrian
--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer - glaubitz@debian.org
`. `' Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
^ permalink raw reply
* [PATCH] serial: cpm_uart: Protect udbg definitions by CONFIG_SERIAL_CPM_CONSOLE
From: Geert Uytterhoeven @ 2021-10-27 7:53 UTC (permalink / raw)
To: Christophe Leroy, Greg Kroah-Hartman, Jiri Slaby
Cc: Geert Uytterhoeven, linuxppc-dev, linux-serial, Arnd Bergmann,
linux-kernel
If CONFIG_CONSOLE_POLL=y, and CONFIG_SERIAL_CPM=m (hence
CONFIG_SERIAL_CPM_CONSOLE=n):
drivers/tty/serial/cpm_uart/cpm_uart_core.c:1109:12: warning: ‘udbg_cpm_getc’ defined but not used [-Wunused-function]
1109 | static int udbg_cpm_getc(void)
| ^~~~~~~~~~~~~
drivers/tty/serial/cpm_uart/cpm_uart_core.c:1095:13: warning: ‘udbg_cpm_putc’ defined but not used [-Wunused-function]
1095 | static void udbg_cpm_putc(char c)
| ^~~~~~~~~~~~~
Fix this by making the udbg definitions depend on
CONFIG_SERIAL_CPM_CONSOLE, in addition to CONFIG_CONSOLE_POLL.
Fixes: a60526097f42eb98 ("tty: serial: cpm_uart: Add udbg support for enabling xmon")
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
One more casualty of CONFIG_WERROR=y.
http://kisskb.ellerman.id.au/kisskb/buildresult/14652935/
---
drivers/tty/serial/cpm_uart/cpm_uart_core.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/tty/serial/cpm_uart/cpm_uart_core.c b/drivers/tty/serial/cpm_uart/cpm_uart_core.c
index c719aa2b18328321..d6d3db9c3b1f83ab 100644
--- a/drivers/tty/serial/cpm_uart/cpm_uart_core.c
+++ b/drivers/tty/serial/cpm_uart/cpm_uart_core.c
@@ -1090,6 +1090,7 @@ static void cpm_put_poll_char(struct uart_port *port,
cpm_uart_early_write(pinfo, ch, 1, false);
}
+#ifdef CONFIG_SERIAL_CPM_CONSOLE
static struct uart_port *udbg_port;
static void udbg_cpm_putc(char c)
@@ -1114,6 +1115,7 @@ static int udbg_cpm_getc(void)
cpu_relax();
return c;
}
+#endif /* CONFIG_SERIAL_CPM_CONSOLE */
#endif /* CONFIG_CONSOLE_POLL */
--
2.25.1
^ permalink raw reply related
* [PATCH] usb: gadget: Mark USB_FSL_QE broken on 64-bit
From: Geert Uytterhoeven @ 2021-10-27 8:08 UTC (permalink / raw)
To: Li Yang, Felipe Balbi, Greg Kroah-Hartman
Cc: Geert Uytterhoeven, linux-usb, linuxppc-dev, linux-kernel,
Arnd Bergmann
On 64-bit:
drivers/usb/gadget/udc/fsl_qe_udc.c: In function ‘qe_ep0_rx’:
drivers/usb/gadget/udc/fsl_qe_udc.c:842:13: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
842 | vaddr = (u32)phys_to_virt(in_be32(&bd->buf));
| ^
In file included from drivers/usb/gadget/udc/fsl_qe_udc.c:41:
drivers/usb/gadget/udc/fsl_qe_udc.c:843:28: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
843 | frame_set_data(pframe, (u8 *)vaddr);
| ^
The driver assumes physical and virtual addresses are 32-bit, hence it
cannot work on 64-bit platforms.
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
One more casualty of CONFIG_WERROR=y.
http://kisskb.ellerman.id.au/kisskb/buildresult/14652936/
---
drivers/usb/gadget/udc/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/gadget/udc/Kconfig b/drivers/usb/gadget/udc/Kconfig
index 8c614bb86c665c77..69394dc1cdfb6436 100644
--- a/drivers/usb/gadget/udc/Kconfig
+++ b/drivers/usb/gadget/udc/Kconfig
@@ -330,6 +330,7 @@ config USB_AMD5536UDC
config USB_FSL_QE
tristate "Freescale QE/CPM USB Device Controller"
depends on FSL_SOC && (QUICC_ENGINE || CPM)
+ depends on !64BIT || BROKEN
help
Some of Freescale PowerPC processors have a Full Speed
QE/CPM2 USB controller, which support device mode with 4
--
2.25.1
^ permalink raw reply related
* Re: [PATCH] usb: gadget: Mark USB_FSL_QE broken on 64-bit
From: Arnd Bergmann @ 2021-10-27 10:38 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Felipe Balbi, Arnd Bergmann, Greg Kroah-Hartman, USB list,
Linux Kernel Mailing List, Li Yang, linuxppc-dev
In-Reply-To: <20211027080849.3276289-1-geert@linux-m68k.org>
On Wed, Oct 27, 2021 at 10:08 AM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
> On 64-bit:
>
> drivers/usb/gadget/udc/fsl_qe_udc.c: In function ‘qe_ep0_rx’:
> drivers/usb/gadget/udc/fsl_qe_udc.c:842:13: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
> 842 | vaddr = (u32)phys_to_virt(in_be32(&bd->buf));
> | ^
> In file included from drivers/usb/gadget/udc/fsl_qe_udc.c:41:
> drivers/usb/gadget/udc/fsl_qe_udc.c:843:28: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
> 843 | frame_set_data(pframe, (u8 *)vaddr);
> | ^
>
> The driver assumes physical and virtual addresses are 32-bit, hence it
> cannot work on 64-bit platforms.
>
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
Interesting, I have not seen this one in randconfig builds.
It looks like the driver also assumes that physical addresses are the same as
bus addresses, so maybe it should also be marked broken when CONFIG_IOMMU
is enabled? Maybe that takes it too far, as this driver could still be used
on a machine without IOMMU in a kernel that supports IOMMUs on
other machines.
Arnd
^ permalink raw reply
* Re: [PATCH] MAINTAINERS: Update powerpc KVM entry
From: Paolo Bonzini @ 2021-10-27 10:58 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev, paulus
Cc: kvm, kvm-ppc, npiggin, linux-kernel
In-Reply-To: <20211027061646.540708-1-mpe@ellerman.id.au>
On 27/10/21 08:16, Michael Ellerman wrote:
> Paul is no longer handling patches for kvmppc.
>
> Instead we'll treat them as regular powerpc patches, taking them via the
> powerpc tree, using the topic/ppc-kvm branch when necessary.
>
> Also drop the web reference, it doesn't have any information
> specifically relevant to powerpc KVM.
>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
> MAINTAINERS | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index ca6d6fde85cf..fbfd3345c40d 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10260,11 +10260,8 @@ F: arch/mips/include/uapi/asm/kvm*
> F: arch/mips/kvm/
>
> KERNEL VIRTUAL MACHINE FOR POWERPC (KVM/powerpc)
> -M: Paul Mackerras <paulus@ozlabs.org>
> -L: kvm-ppc@vger.kernel.org
> -S: Supported
> -W: http://www.linux-kvm.org/
> -T: git git://github.com/agraf/linux-2.6.git
> +L: linuxppc-dev@lists.ozlabs.org
> +T: git git://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git topic/ppc-kvm
> F: arch/powerpc/include/asm/kvm*
> F: arch/powerpc/include/uapi/asm/kvm*
> F: arch/powerpc/kernel/kvm*
>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Thanks Michael and Paul!
Paolo
^ permalink raw reply
* Re: Linux kernel: powerpc: KVM guest can trigger host crash on Power8
From: Michael Ellerman @ 2021-10-27 11:06 UTC (permalink / raw)
To: John Paul Adrian Glaubitz
Cc: oss-security, debian-powerpc@lists.debian.org, linuxppc-dev
In-Reply-To: <04864fe5-fdd0-74b2-2bad-0303e4c2b15a@physik.fu-berlin.de>
John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de> writes:
> Hi Michael!
>
> On 10/27/21 07:30, Michael Ellerman wrote:
>> I did test the repro case you gave me before (in the bugzilla), which
>> was building glibc, that passes for me with a patched host.
>
> Did you manage to crash the unpatched host?
Yes, the parallel builds of glibc you described crashed the unpatched
host 100% reliably for me.
I also have a standalone reproducer I'll send you.
> If the unpatched host crashes for you but the patched doesn't, I will
> make sure I didn't accidentally miss anything.
OK thanks.
> Also, I'll try a kernel from git with Debian's config.
>
>> I guess we have yet another bug.
>>
>> I tried the following in a debian BE VM and it completed fine:
>>
>> $ dget -u http://ftp.debian.org/debian/pool/main/g/git/git_2.33.1-1.dsc
>> $ sbuild -d sid --arch=powerpc --no-arch-all git_2.33.1-1.dsc
>>
>> Same for ppc64.
>>
>> And I also tried both at once, repeatedly in a loop.
>
> Did you try building gcc-11 for powerpc and ppc64 both at once?
No, I will try that now.
>> I guess it's something more complicated.
>>
>> What exact host/guest kernel versions and configs are you running?
>
> Both the host and guest are running Debian's stock 5.14.12 kernel. The host has
> a kernel with your patches applied, the guest doesn't.
OK that sounds fine.
I tested upstream stable v5.14.13 + my patches, but there's nothing
betwen 5.14.12 and 5.14.13 that should matter AFAICS.
> Let me do some more testing.
Thanks.
cheers
^ permalink raw reply
* Re: Linux kernel: powerpc: KVM guest can trigger host crash on Power8
From: John Paul Adrian Glaubitz @ 2021-10-27 11:09 UTC (permalink / raw)
To: Michael Ellerman
Cc: oss-security, debian-powerpc@lists.debian.org, linuxppc-dev
In-Reply-To: <874k92bubv.fsf@mpe.ellerman.id.au>
Hi Michael!
On 10/27/21 13:06, Michael Ellerman wrote:
> John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de> writes:
>> Hi Michael!
>>
>> On 10/27/21 07:30, Michael Ellerman wrote:
>>> I did test the repro case you gave me before (in the bugzilla), which
>>> was building glibc, that passes for me with a patched host.
>>
>> Did you manage to crash the unpatched host?
>
> Yes, the parallel builds of glibc you described crashed the unpatched
> host 100% reliably for me.
OK, that is very good news!
> I also have a standalone reproducer I'll send you.
Thanks, that would be helpful!
>> Also, I'll try a kernel from git with Debian's config.
>>
>>> I guess we have yet another bug.
>>>
>>> I tried the following in a debian BE VM and it completed fine:
>>>
>>> $ dget -u http://ftp.debian.org/debian/pool/main/g/git/git_2.33.1-1.dsc
>>> $ sbuild -d sid --arch=powerpc --no-arch-all git_2.33.1-1.dsc
>>>
>>> Same for ppc64.
>>>
>>> And I also tried both at once, repeatedly in a loop.
>>
>> Did you try building gcc-11 for powerpc and ppc64 both at once?
>
> No, I will try that now.
OK, great!
Adrian
--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer - glaubitz@debian.org
`. `' Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
^ permalink raw reply
* Re: [PATCH][next] powerpc/vas: Fix potential NULL pointer dereference
From: Michael Ellerman @ 2021-10-27 11:21 UTC (permalink / raw)
To: Gustavo A. R. Silva
Cc: Tyrel Datwyler, Haren Myneni, linux-kernel, Nicholas Piggin,
Paul Mackerras, linux-hardening, linuxppc-dev
In-Reply-To: <20211026224016.GA1488461@embeddedor>
"Gustavo A. R. Silva" <gustavoars@kernel.org> writes:
> On Wed, Oct 27, 2021 at 09:30:53AM +1100, Michael Ellerman wrote:
> [..]
>> > I think I'll take this in my tree.
>>
>> I've already put it in powerpc/next:
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?h=next&id=61cb9ac66b30374c7fd8a8b2a3c4f8f432c72e36
>
> Oh, great. :)
>
>> If you need to pick it up as well for some reason that's fine.
>
> I just didn't want it to get lost somehow. I'll drop it from tree now.
No worries, sorry I've been so slow lately.
cheers
^ permalink raw reply
* Re: linux-next: manual merge of the audit tree with the powerpc tree
From: Michael Ellerman @ 2021-10-27 11:29 UTC (permalink / raw)
To: Paul Moore
Cc: Stephen Rothwell, Richard Guy Briggs, Linux Kernel Mailing List,
Linux Next Mailing List, PowerPC
In-Reply-To: <CAHC9VhTj7gn3iAOYctVRKvv_Bk1iQMrmkA8FVJtYzdvBjqFmvg@mail.gmail.com>
Paul Moore <paul@paul-moore.com> writes:
> On Tue, Oct 26, 2021 at 6:55 AM Michael Ellerman <mpe@ellerman.id.au> wrote:
>> Stephen Rothwell <sfr@canb.auug.org.au> writes:
>> > Hi all,
>> >
>> > Today's linux-next merge of the audit tree got conflicts in:
>> >
>> > arch/powerpc/kernel/audit.c
>> > arch/powerpc/kernel/compat_audit.c
>> >
>> > between commit:
>> >
>> > 566af8cda399 ("powerpc/audit: Convert powerpc to AUDIT_ARCH_COMPAT_GENERIC")
>> >
>> > from the powerpc tree and commits:
>> >
>> > 42f355ef59a2 ("audit: replace magic audit syscall class numbers with macros")
>> > 1c30e3af8a79 ("audit: add support for the openat2 syscall")
>> >
>> > from the audit tree.
>>
>> Thanks.
>>
>> I guess this is OK, unless the audit folks disagree. I could revert the
>> powerpc commit and try it again later.
>>
>> If I don't hear anything I'll leave it as-is.
>
> Hi Michael,
>
> Last I recall from the powerpc/audit thread there were still some
> issues with audit working properly in your testing, has that been
> resolved?
No.
There's one test failure both before and after the conversion to use the
generic code.
> If nothing else, -rc7 seems a bit late for this to hit -next for me to
> feel comfortable about this.
OK. I'll revert the patch in my tree.
cheers
^ permalink raw reply
* Re: linux-next: manual merge of the audit tree with the powerpc tree
From: Christophe Leroy @ 2021-10-27 11:41 UTC (permalink / raw)
To: Michael Ellerman, Paul Moore
Cc: Richard Guy Briggs, Stephen Rothwell, Linux Next Mailing List,
PowerPC, Linux Kernel Mailing List
In-Reply-To: <87tuh2aepp.fsf@mpe.ellerman.id.au>
Le 27/10/2021 à 13:29, Michael Ellerman a écrit :
> Paul Moore <paul@paul-moore.com> writes:
>> On Tue, Oct 26, 2021 at 6:55 AM Michael Ellerman <mpe@ellerman.id.au> wrote:
>>> Stephen Rothwell <sfr@canb.auug.org.au> writes:
>>>> Hi all,
>>>>
>>>> Today's linux-next merge of the audit tree got conflicts in:
>>>>
>>>> arch/powerpc/kernel/audit.c
>>>> arch/powerpc/kernel/compat_audit.c
>>>>
>>>> between commit:
>>>>
>>>> 566af8cda399 ("powerpc/audit: Convert powerpc to AUDIT_ARCH_COMPAT_GENERIC")
>>>>
>>>> from the powerpc tree and commits:
>>>>
>>>> 42f355ef59a2 ("audit: replace magic audit syscall class numbers with macros")
>>>> 1c30e3af8a79 ("audit: add support for the openat2 syscall")
>>>>
>>>> from the audit tree.
>>>
>>> Thanks.
>>>
>>> I guess this is OK, unless the audit folks disagree. I could revert the
>>> powerpc commit and try it again later.
>>>
>>> If I don't hear anything I'll leave it as-is.
>>
>> Hi Michael,
>>
>> Last I recall from the powerpc/audit thread there were still some
>> issues with audit working properly in your testing, has that been
>> resolved?
>
> No.
>
> There's one test failure both before and after the conversion to use the
> generic code.
>
>> If nothing else, -rc7 seems a bit late for this to hit -next for me to
>> feel comfortable about this.
>
> OK. I'll revert the patch in my tree.
>
But it's been in the pipe since end of August and no one reported any
issue other issue than the pre-existing one, so what's the new issue
that prevents us to merge it two monthes later, and how do we walk
forward then ?
Thanks
Christophe
^ permalink raw reply
* Re: [PATCH] locking: remove spin_lock_flags() etc
From: Michael Ellerman @ 2021-10-27 12:01 UTC (permalink / raw)
To: Peter Zijlstra, Arnd Bergmann
Cc: linux-ia64, James E.J. Bottomley, Paul Mackerras,
Alexander Gordeev, Will Deacon, Jonas Bonn, linux-s390,
Arnd Bergmann, Helge Deller, Christian Borntraeger, Ingo Molnar,
Waiman Long, Heiko Carstens, Vasily Gorbik, Boqun Feng,
Stefan Kristiansson, Openrisc, Stafford Horne, Parisc List,
Linux Kernel Mailing List, linuxppc-dev
In-Reply-To: <YXbAPIm47WwpYYup@hirez.programming.kicks-ass.net>
Peter Zijlstra <peterz@infradead.org> writes:
> On Mon, Oct 25, 2021 at 03:06:24PM +0200, Arnd Bergmann wrote:
>> On Mon, Oct 25, 2021 at 11:57 AM Peter Zijlstra <peterz@infradead.org> wrote:
>> > On Sat, Oct 23, 2021 at 06:04:57PM +0200, Arnd Bergmann wrote:
>> > > On Sat, Oct 23, 2021 at 3:37 AM Waiman Long <longman@redhat.com> wrote:
>> > > >> On 10/22/21 7:59 AM, Arnd Bergmann wrote:
>> > > > > From: Arnd Bergmann <arnd@arndb.de>
>> > > > >
>> > > > > As this is all dead code, just remove it and the helper functions built
>> > > > > around it. For arch/ia64, the inline asm could be cleaned up, but
>> > > > > it seems safer to leave it untouched.
>> > > > >
>> > > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> > > >
>> > > > Does that mean we can also remove the GENERIC_LOCKBREAK config option
>> > > > from the Kconfig files as well?
>> > >
>> > > I couldn't figure this out.
>> > >
>> > > What I see is that the only architectures setting GENERIC_LOCKBREAK are
>> > > nds32, parisc, powerpc, s390, sh and sparc64, while the only architectures
>> > > implementing arch_spin_is_contended() are arm32, csky and ia64.
>> > >
>> > > The part I don't understand is whether the option actually does anything
>> > > useful any more after commit d89c70356acf ("locking/core: Remove break_lock
>> > > field when CONFIG_GENERIC_LOCKBREAK=y").
>> >
>> > Urgh, what a mess.. AFAICT there's still code in
>> > kernel/locking/spinlock.c that relies on it. Specifically when
>> > GENERIC_LOCKBREAK=y we seem to create _lock*() variants that are
>> > basically TaS locks which drop preempt/irq disable while spinning.
>> >
>> > Anybody having this on and not having native TaS locks is in for a rude
>> > surprise I suppose... sparc64 being the obvious candidate there :/
>>
>> Is this a problem on s390 and powerpc, those two being the ones
>> that matter in practice?
>>
>> On s390, we pick between the cmpxchg() based directed-yield when
>> running on virtualized CPUs, and a normal qspinlock when running on a
>> dedicated CPU.
>>
>> On PowerPC, we pick at compile-time between either the qspinlock
>> (default-enabled on Book3S-64, i.e. all server chips) or a ll/sc based
>> spinlock plus vm_yield() (default on embedded and 32-bit mac).
>
> Urgh, yeah, so this crud undermines the whole point of having a fair
> lock. I'm thinking s390 and Power want to have this fixed.
Our Kconfig has:
config GENERIC_LOCKBREAK
bool
default y
depends on SMP && PREEMPTION
And we have exactly one defconfig that enables both SMP and PREEMPT,
arch/powerpc/configs/85xx/ge_imp3a_defconfig, which is some ~10 year old
PCI card embedded thing I've never heard of. High chance anyone who has
those is not running upstream kernels on them.
So I think we'd be happy for you rip GENERIC_LOCKBREAK out, it's almost
entirely unused on powerpc anyway.
cheers
^ permalink raw reply
* Re: instruction storage exception handling
From: Jacques de Laval @ 2021-10-27 12:03 UTC (permalink / raw)
To: Christophe Leroy
Cc: Scott Wood, linuxppc-dev@lists.ozlabs.org, Nicholas Piggin
In-Reply-To: <f5824237-4fd4-ca87-afe7-620a23d84824@csgroup.eu>
On Wednesday, October 27th, 2021 at 9:52 AM, Christophe Leroy <christophe.leroy@csgroup.eu> wrote:
> Le 27/10/2021 à 09:47, Nicholas Piggin a écrit :
>
> > Excerpts from Christophe Leroy's message of October 27, 2021 3:51 pm:
> >
> > > Le 27/10/2021 à 07:25, Nicholas Piggin a écrit :
> > >
> > > > Excerpts from Christophe Leroy's message of October 27, 2021 3:00 pm:
> > > >
> > > > > Le 27/10/2021 à 06:10, Nicholas Piggin a écrit :
> > > > >
> > > > > > Excerpts from Jacques de Laval's message of October 26, 2021 6:07 am:
> > > > > >
> > > > > > > Hi,
> > > > > > >
> > > > > > > We are trying to upgrade kernel from 5.10 to 5.14.11. We have a Freescale/NXP
> > > > > > >
> > > > > > > T1023 SOC with two e5500 cores, and are running in 32-bit mode:
> > > > > > >
> > > > > > > CONFIG_PPC32=y
> > > > > > >
> > > > > > > CONFIG_PPC64 is not set
> > > > > > > =======================
> > > > > > >
> > > > > > > Processor support
> > > > > > > =================
> > > > > > >
> > > > > > > CONFIG_PPC_BOOK3S_32 is not set
> > > > > > > ===============================
> > > > > > >
> > > > > > > CONFIG_PPC_85xx=y
> > > > > > >
> > > > > > > CONFIG_PPC_8xx is not set
> > > > > > > =========================
> > > > > > >
> > > > > > > CONFIG_40x is not set
> > > > > > > =====================
> > > > > > >
> > > > > > > CONFIG_44x is not set
> > > > > > > =====================
> > > > > > >
> > > > > > > CONFIG_GENERIC_CPU=y
> > > > > > >
> > > > > > > CONFIG_E5500_CPU is not set
> > > > > > > ===========================
> > > > > > >
> > > > > > > CONFIG_E6500_CPU is not set
> > > > > > > ===========================
> > > > > > >
> > > > > > > CONFIG_E500=y
> > > > > > >
> > > > > > > CONFIG_PPC_E500MC=y
> > > > > > >
> > > > > > > CONFIG_PPC_FPU_REGS=y
> > > > > > >
> > > > > > > CONFIG_PPC_FPU=y
> > > > > > >
> > > > > > > CONFIG_FSL_EMB_PERFMON=y
> > > > > > >
> > > > > > > CONFIG_FSL_EMB_PERF_EVENT=y
> > > > > > >
> > > > > > > CONFIG_FSL_EMB_PERF_EVENT_E500=y
> > > > > > >
> > > > > > > CONFIG_BOOKE=y
> > > > > > >
> > > > > > > CONFIG_FSL_BOOKE=y
> > > > > > >
> > > > > > > CONFIG_PPC_FSL_BOOK3E=y
> > > > > > >
> > > > > > > CONFIG_PTE_64BIT=y
> > > > > > >
> > > > > > > CONFIG_PHYS_64BIT=y
> > > > > > >
> > > > > > > CONFIG_PPC_MMU_NOHASH=y
> > > > > > >
> > > > > > > CONFIG_PPC_BOOK3E_MMU=y
> > > > > > >
> > > > > > > CONFIG_PMU_SYSFS is not set
> > > > > > > ===========================
> > > > > > >
> > > > > > > CONFIG_SMP=y
> > > > > > >
> > > > > > > CONFIG_NR_CPUS=2
> > > > > > >
> > > > > > > CONFIG_PPC_DOORBELL=y
> > > > > > >
> > > > > > > end of Processor support
> > > > > > > ========================
> > > > > > >
> > > > > > > We compile using 32-bit Bootlin PPC toolchain:
> > > > > > >
> > > > > > > powerpc-e500mc glibc bleeding-edge 2020.08-1.
> > > > > > >
> > > > > > > When booting, and starting PID 1 we sometimes get a hang. Nothing but our init
> > > > > > >
> > > > > > > process is running, and for debugging purposes our init currently looks like
> > > > > > >
> > > > > > > this:
> > > > > > >
> > > > > > > int main(int argc, char *argv[]){
> > > > > > >
> > > > > > > for (int i = 0; ; i++) {
> > > > > > >
> > > > > > > FILE *fp = fopen("/dev/kmsg", "w");
> > > > > > >
> > > > > > > if (fp) {
> > > > > > >
> > > > > > > fprintf(fp, "%d\n", i);
> > > > > > >
> > > > > > > fclose(fp);
> > > > > > >
> > > > > > > }
> > > > > > >
> > > > > > > sleep(1);
> > > > > > >
> > > > > > > }
> > > > > > >
> > > > > > > return 0;
> > > > > > >
> > > > > > > }
> > > > > > >
> > > > > > > When the hangup occur we don't get any output at all from our PID 1.
> > > > > > >
> > > > > > > The last output is from the kernel:
> > > > > > >
> > > > > > > Run /sbin/init as init process
> > > > > > >
> > > > > > > with arguments:
> > > > > > >
> > > > > > > /sbin/init
> > > > > > >
> > > > > > > with environment:
> > > > > > >
> > > > > > > HOME=/
> > > > > > >
> > > > > > > TERM=linux
> > > > > > >
> > > > > > > kgdboc=ttyS0,115200
> > > > > > >
> > > > > > > When issuing a backtrace on all active cpus we can see that the kernel is
> > > > > > >
> > > > > > > handling an instruction storage exception:
> > > > > > >
> > > > > > > sysrq: Show backtrace of all active CPUs
> > > > > > >
> > > > > > > sysrq: CPU0:
> > > > > > >
> > > > > > > CPU: 0 PID: 1 Comm: init Not tainted 5.14.11 #1
> > > > > > >
> > > > > > > NIP: c02aac78 LR: c02aac2c CTR: 00000000
> > > > > > >
> > > > > > > REGS: c1907d40 TRAP: 0500 Not tainted (5.14.11)
> > > > > > >
> > > > > > > MSR: 00029002 <CE,EE,ME> CR: 82244284 XER: 20000000
> > > > > > >
> > > > > > > GPR00: 0000000f c1907e20 c1910000 0065a000 000001d0 01100cca c1907e84 0000000c
> > > > > > >
> > > > > > > GPR08: d39a8000 000001d3 0000000c c1907f10 42244284 00000000 00740514 bfb71670
> > > > > > >
> > > > > > > GPR16: 007040e6 00701418 b7c1a5f0 00702f18 00000000 bfb71690 0000fff1 b7c1c478
> > > > > > >
> > > > > > > GPR24: 00708558 00701698 d3994040 00029002 c1907f20 0065a238 00000355 d39a2790
> > > > > > >
> > > > > > > NIP [c02aac78] handle_mm_fault+0xf8/0x11f0
> > > > > > >
> > > > > > > LR [c02aac2c] handle_mm_fault+0xac/0x11f0
> > > > > > >
> > > > > > > Call Trace:
> > > > > > >
> > > > > > > [c1907e20] [c02aac10] handle_mm_fault+0x90/0x11f0 (unreliable)
> > > > > > >
> > > > > > > [c1907ec0] [c003078c] ___do_page_fault+0x26c/0x780
> > > > > > >
> > > > > > > [c1907ef0] [c0030cd4] do_page_fault+0x34/0x100
> > > > > > >
> > > > > > > [c1907f10] [c0000988] InstructionStorage+0x108/0x120
> > > > > > >
> > > > > > > --- interrupt: 400 at 0x65a238
> > > > > > >
> > > > > > > NIP: 0065a238 LR: 0052f26c CTR: 0052f260
> > > > > > >
> > > > > > > REGS: c1907f20 TRAP: 0400 Not tainted (5.14.11)
> > > > > > >
> > > > > > > MSR: 0002d002 <CE,EE,PR,ME> CR: 42242284 XER: 00000000
> > > > > > >
> > > > > > > GPR00: b7be9914 bfb71620 b7c203a0 8c008000 0070400d b7c182a0 000b8260 0052f260
> > > > > > >
> > > > > > > GPR08: 0047d448 0052f260 0000000a 00000003 42242284 00000000 00740514 bfb71670
> > > > > > >
> > > > > > > GPR16: 007040e6 00701418 b7c1a5f0 00702f18 00000000 bfb71690 0000fff1 b7c1c478
> > > > > > >
> > > > > > > GPR24: 00708558 00701698 00700000 00000015 b7c1c2b0 00707e20 b7c1b8a8 bfb71660
> > > > > > >
> > > > > > > NIP [0065a238] 0x65a238
> > > > > > >
> > > > > > > LR [0052f26c] 0x52f26c
> > > > > > >
> > > > > > > --- interrupt: 400
> > > > > > >
> > > > > > > Instruction dump:
> > > > > > >
> > > > > > > 60a500c0 811f0020 57aa6cfa 813f0000 57a30026 809f004c 81080024 7d29e850
> > > > > > >
> > > > > > > 90a1002c 5529a33e 93c10038 7d244a14 <90610034> 7d485215 91210030 41c203dc
> > > > > > >
> > > > > > > We have also observed that the CPU is continuously servicing the same interrupt
> > > > > > >
> > > > > > > (north of 140k times per sec), it is not deadlocked.
> > > > > > >
> > > > > > > We have not yet been able to reproduce this behavior under QEMU system
> > > > > > >
> > > > > > > emulation.
> > > > > > >
> > > > > > > When bisecting between 5.10 and 5.14.11 we can see that this behavior started
> > > > > > >
> > > > > > > with commit a01a3f2ddbcda83e8572787c0ec1dcbeba86915a:
> > > > > > >
> > > > > > > powerpc: remove arguments from fault handler functions
> > > > > >
> > > > > > Thank you for the excellent work to investigate and report this.
> > > > > >
> > > > > > > Our best guess that the instruction storage exception is not properly handled
> > > > > > >
> > > > > > > and the kernel is never able to recover from the page fault, but we don't
> > > > > > >
> > > > > > > really know how to proceed. Does anyone have any suggestions or insights?
> > > > > >
> > > > > > Before my patch, zero was passed to the page fault error code, after
> > > > > >
> > > > > > my patch it passes the contents of ESR SPR.
> > > > > >
> > > > > > It looks like the BookE instruction access interrupt does not set ESR
> > > > > >
> > > > > > (except for BO interrupts maybe?) so you're getting what was in the ESR
> > > > > >
> > > > > > register from a previous interrupt, and maybe if that was a store then
> > > > > >
> > > > > > access_error won't cause a segfault because is_exec is true so that
> > > > > >
> > > > > > test bails out early, then it might just keep retrying the interrupt.
> > > > > >
> > > > > > That could explain why you don't always see the same thing.
> > > > > >
> > > > > > Now previous code still saved ESR in regs->esr/dsisr for some reason.
> > > > > >
> > > > > > I can't quite see why that should have been necessary though. Does
> > >
> > > According to the e500 Reference Manual, on ISI:
> > >
> > > BO is set if the instruction fetch caused a byte-ordering exception;
> > >
> > > otherwise cleared. All other defined ESR bits are cleared.
> >
> > You're right. In that case it shouldn't change anything unless there
> >
> > was a BO fault. I'm not sure what the problem is then. Guessing based
> >
> > on the NIP and instructions, it looks like it's probably got the correct
> >
> > user address that it's storing into vmf on the stack, so it has got past
> >
> > the access checks so my theory would be wrong anyway.
> >
> > Must be something simple but I can't see it yet.
>
> Anyway, I think it is still worth doing the check with setting 0 in
>
> _ESR(r11), maybe the Reference Manual is wrong.
>
> So Jacques, please do the test anyway if you can.
>
> Thanks
>
> Christophe
I tested with the last patch from Nicholas, and with that I can not
reproduce the issue, so it seems like that solves it for my case and setup
at least.
Big thanks Christophe and Nicholas for looking in to this!
Thanks,
Jacques
^ permalink raw reply
* Re: [PATCH] powerpc/xmon: fix task state output
From: Denis Kirjanov @ 2021-10-27 12:36 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <87bl3bb7bg.fsf@mpe.ellerman.id.au>
On 10/27/21, Michael Ellerman <mpe@ellerman.id.au> wrote:
> Denis Kirjanov <kda@linux-powerpc.org> writes:
>> p_state is unsigned since the commit 2f064a59a11f
>>
>> The patch also uses TASK_RUNNING instead of null.
>>
>> Fixes: 2f064a59a11f ("sched: Change task_struct::state")
>> Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
>> ---
>> arch/powerpc/xmon/xmon.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
>> index dd8241c009e5..8b28ff9d98d1 100644
>> --- a/arch/powerpc/xmon/xmon.c
>> +++ b/arch/powerpc/xmon/xmon.c
>> @@ -3264,8 +3264,7 @@ static void show_task(struct task_struct *volatile
>> tsk)
>> * appropriate for calling from xmon. This could be moved
>> * to a common, generic, routine used by both.
>> */
>> - state = (p_state == 0) ? 'R' :
>> - (p_state < 0) ? 'U' :
>
> I guess 'U' meant 'unknown'? I always thought it meant uninterruptible,
> but obviously that is 'D'.
Right.
>
>> + state = (p_state == TASK_RUNNING) ? 'R' :
>> (p_state & TASK_UNINTERRUPTIBLE) ? 'D' :
>> (p_state & TASK_STOPPED) ? 'T' :
>> (p_state & TASK_TRACED) ? 'C' :
>
> I think a better cleanup would be to use task_is_running(),
> task_is_traced(), task_is_stopped(). That way we're insulated somewhat
> from any future changes.
In this case we end up with some states wrapped with a macro but
others are still explicitly checked using p_state variable.
>
> That would add additional READ_ONCE()s of the state, but I don't think
> we care, the task should not be running if the system is in xmon.
>
> cheers
>
^ permalink raw reply
* [PATCH 2/3] fbdev: rework backlight dependencies
From: Arnd Bergmann @ 2021-10-27 13:27 UTC (permalink / raw)
To: dri-devel
Cc: linux-fbdev, linux-staging, Arnd Bergmann, Jens Frederich,
intel-gfx, Randy Dunlap, Lars Poeschel, linux-kernel, Jani Nikula,
Daniel Vetter, Andy Shevchenko, Geert Uytterhoeven,
Thomas Zimmermann, Greg Kroah-Hartman, Miguel Ojeda,
Robin van der Gracht, linuxppc-dev, Jon Nettleton
In-Reply-To: <20211027132732.3993279-1-arnd@kernel.org>
From: Arnd Bergmann <arnd@arndb.de>
Rather than having CONFIG_FB_BACKLIGHT select CONFIG_BACKLIGHT_CLASS_DEVICE,
make any driver that needs it have a dependency on the class device
being available, to prevent circular dependencies.
This is the same way that the backlight is already treated for the DRM
subsystem.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/auxdisplay/Kconfig | 1 +
drivers/macintosh/Kconfig | 1 +
drivers/staging/fbtft/Kconfig | 1 +
drivers/staging/olpc_dcon/Kconfig | 2 +-
drivers/video/fbdev/Kconfig | 14 +++++++++++---
5 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/auxdisplay/Kconfig b/drivers/auxdisplay/Kconfig
index 64012cda4d12..356afb6a569d 100644
--- a/drivers/auxdisplay/Kconfig
+++ b/drivers/auxdisplay/Kconfig
@@ -170,6 +170,7 @@ config IMG_ASCII_LCD
config HT16K33
tristate "Holtek Ht16K33 LED controller with keyscan"
depends on FB && I2C && INPUT
+ depends on BACKLIGHT_CLASS_DEVICE
select FB_SYS_FOPS
select FB_SYS_FILLRECT
select FB_SYS_COPYAREA
diff --git a/drivers/macintosh/Kconfig b/drivers/macintosh/Kconfig
index 5cdc361da37c..cb0fbdf8ff7f 100644
--- a/drivers/macintosh/Kconfig
+++ b/drivers/macintosh/Kconfig
@@ -113,6 +113,7 @@ config PMAC_MEDIABAY
config PMAC_BACKLIGHT
bool "Backlight control for LCD screens"
depends on PPC_PMAC && ADB_PMU && FB = y && (BROKEN || !PPC64)
+ depends on BACKLIGHT_CLASS_DEVICE
select FB_BACKLIGHT
help
Say Y here to enable Macintosh specific extensions of the generic
diff --git a/drivers/staging/fbtft/Kconfig b/drivers/staging/fbtft/Kconfig
index dad1ddcd7b0c..c4f2f01cd798 100644
--- a/drivers/staging/fbtft/Kconfig
+++ b/drivers/staging/fbtft/Kconfig
@@ -3,6 +3,7 @@ menuconfig FB_TFT
tristate "Support for small TFT LCD display modules"
depends on FB && SPI
depends on GPIOLIB || COMPILE_TEST
+ depends on BACKLIGHT_CLASS_DEVICE
select FB_SYS_FILLRECT
select FB_SYS_COPYAREA
select FB_SYS_IMAGEBLIT
diff --git a/drivers/staging/olpc_dcon/Kconfig b/drivers/staging/olpc_dcon/Kconfig
index d1a0dea09ef0..a9f36538d7ab 100644
--- a/drivers/staging/olpc_dcon/Kconfig
+++ b/drivers/staging/olpc_dcon/Kconfig
@@ -4,7 +4,7 @@ config FB_OLPC_DCON
depends on OLPC && FB
depends on I2C
depends on GPIO_CS5535 && ACPI
- select BACKLIGHT_CLASS_DEVICE
+ depends on BACKLIGHT_CLASS_DEVICE
help
In order to support very low power operation, the XO laptop uses a
secondary Display CONtroller, or DCON. This secondary controller
diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index c187a93c9a16..3ad499433726 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -186,7 +186,7 @@ config FB_MACMODES
config FB_BACKLIGHT
tristate
depends on FB
- select BACKLIGHT_CLASS_DEVICE
+ depends on BACKLIGHT_CLASS_DEVICE
config FB_MODE_HELPERS
bool "Enable Video Mode Handling Helpers"
@@ -275,12 +275,12 @@ config FB_ARMCLCD
tristate "ARM PrimeCell PL110 support"
depends on ARM || ARM64 || COMPILE_TEST
depends on FB && ARM_AMBA && HAS_IOMEM
+ depends on BACKLIGHT_CLASS_DEVICE || !OF
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
select FB_MODE_HELPERS if OF
select VIDEOMODE_HELPERS if OF
- select BACKLIGHT_CLASS_DEVICE if OF
help
This framebuffer device driver is for the ARM PrimeCell PL110
Colour LCD controller. ARM PrimeCells provide the building
@@ -863,6 +863,7 @@ config FB_ATMEL
tristate "AT91 LCD Controller support"
depends on FB && OF && HAVE_CLK && HAS_IOMEM
depends on HAVE_FB_ATMEL || COMPILE_TEST
+ depends on BACKLIGHT_CLASS_DEVICE
select FB_BACKLIGHT
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
@@ -916,6 +917,7 @@ config FB_NVIDIA_DEBUG
config FB_NVIDIA_BACKLIGHT
bool "Support for backlight control"
depends on FB_NVIDIA
+ depends on BACKLIGHT_CLASS_DEVICE=y || BACKLIGHT_CLASS_DEVICE=FB_NVIDIA
default y
help
Say Y here if you want to control the backlight of your display.
@@ -963,6 +965,7 @@ config FB_RIVA_DEBUG
config FB_RIVA_BACKLIGHT
bool "Support for backlight control"
depends on FB_RIVA
+ depends on BACKLIGHT_CLASS_DEVICE=y || BACKLIGHT_CLASS_DEVICE=FB_RIVA
default y
help
Say Y here if you want to control the backlight of your display.
@@ -1234,6 +1237,7 @@ config FB_RADEON_I2C
config FB_RADEON_BACKLIGHT
bool "Support for backlight control"
depends on FB_RADEON
+ depends on BACKLIGHT_CLASS_DEVICE=y || BACKLIGHT_CLASS_DEVICE=FB_RADEON
default y
help
Say Y here if you want to control the backlight of your display.
@@ -1265,6 +1269,7 @@ config FB_ATY128
config FB_ATY128_BACKLIGHT
bool "Support for backlight control"
depends on FB_ATY128
+ depends on BACKLIGHT_CLASS_DEVICE=y || BACKLIGHT_CLASS_DEVICE=FB_ATY128
default y
help
Say Y here if you want to control the backlight of your display.
@@ -1314,6 +1319,7 @@ config FB_ATY_GX
config FB_ATY_BACKLIGHT
bool "Support for backlight control"
+ depends on BACKLIGHT_CLASS_DEVICE=y || BACKLIGHT_CLASS_DEVICE=FB_ATY
depends on FB_ATY
default y
help
@@ -1838,6 +1844,7 @@ config FB_SH_MOBILE_LCDC
tristate "SuperH Mobile LCDC framebuffer support"
depends on FB && HAVE_CLK && HAS_IOMEM
depends on SUPERH || ARCH_RENESAS || COMPILE_TEST
+ depends on BACKLIGHT_CLASS_DEVICE
select FB_SYS_FILLRECT
select FB_SYS_COPYAREA
select FB_SYS_IMAGEBLIT
@@ -2166,7 +2173,7 @@ config FB_PRE_INIT_FB
config FB_MX3
tristate "MX3 Framebuffer support"
depends on FB && MX3_IPU
- select BACKLIGHT_CLASS_DEVICE
+ depends on BACKLIGHT_CLASS_DEVICE
select FB_CFB_FILLRECT
select FB_CFB_COPYAREA
select FB_CFB_IMAGEBLIT
@@ -2222,6 +2229,7 @@ config FB_SSD1307
tristate "Solomon SSD1307 framebuffer support"
depends on FB && I2C
depends on GPIOLIB || COMPILE_TEST
+ depends on BACKLIGHT_CLASS_DEVICE
select FB_SYS_FOPS
select FB_SYS_FILLRECT
select FB_SYS_COPYAREA
--
2.29.2
^ permalink raw reply related
* Re: [PATCH 2/3] fbdev: rework backlight dependencies
From: Miguel Ojeda @ 2021-10-27 13:32 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Linux Fbdev development list, linux-staging, Arnd Bergmann,
Jens Frederich, Intel Graphics Development, Randy Dunlap,
Lars Poeschel, linux-kernel, Jani Nikula, Daniel Vetter,
Andy Shevchenko, Geert Uytterhoeven, dri-devel, Thomas Zimmermann,
Greg Kroah-Hartman, Miguel Ojeda, Robin van der Gracht,
linuxppc-dev, Jon Nettleton
In-Reply-To: <20211027132732.3993279-2-arnd@kernel.org>
On Wed, Oct 27, 2021 at 3:28 PM Arnd Bergmann <arnd@kernel.org> wrote:
>
> Rather than having CONFIG_FB_BACKLIGHT select CONFIG_BACKLIGHT_CLASS_DEVICE,
> make any driver that needs it have a dependency on the class device
> being available, to prevent circular dependencies.
Acked-by: Miguel Ojeda <ojeda@kernel.org>
Cheers,
Miguel
^ permalink raw reply
* Re: linux-next: manual merge of the audit tree with the powerpc tree
From: Paul Moore @ 2021-10-27 14:18 UTC (permalink / raw)
To: Christophe Leroy
Cc: Stephen Rothwell, Richard Guy Briggs, Linux Kernel Mailing List,
Linux Next Mailing List, PowerPC
In-Reply-To: <2012df5e-62ec-06fb-9f4d-e27dde184a3f@csgroup.eu>
On Wed, Oct 27, 2021 at 7:41 AM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
> Le 27/10/2021 à 13:29, Michael Ellerman a écrit :
> > Paul Moore <paul@paul-moore.com> writes:
> >> On Tue, Oct 26, 2021 at 6:55 AM Michael Ellerman <mpe@ellerman.id.au> wrote:
> >>> Stephen Rothwell <sfr@canb.auug.org.au> writes:
> >>>> Hi all,
> >>>>
> >>>> Today's linux-next merge of the audit tree got conflicts in:
> >>>>
> >>>> arch/powerpc/kernel/audit.c
> >>>> arch/powerpc/kernel/compat_audit.c
> >>>>
> >>>> between commit:
> >>>>
> >>>> 566af8cda399 ("powerpc/audit: Convert powerpc to AUDIT_ARCH_COMPAT_GENERIC")
> >>>>
> >>>> from the powerpc tree and commits:
> >>>>
> >>>> 42f355ef59a2 ("audit: replace magic audit syscall class numbers with macros")
> >>>> 1c30e3af8a79 ("audit: add support for the openat2 syscall")
> >>>>
> >>>> from the audit tree.
> >>>
> >>> Thanks.
> >>>
> >>> I guess this is OK, unless the audit folks disagree. I could revert the
> >>> powerpc commit and try it again later.
> >>>
> >>> If I don't hear anything I'll leave it as-is.
> >>
> >> Hi Michael,
> >>
> >> Last I recall from the powerpc/audit thread there were still some
> >> issues with audit working properly in your testing, has that been
> >> resolved?
> >
> > No.
> >
> > There's one test failure both before and after the conversion to use the
> > generic code.
> >
> >> If nothing else, -rc7 seems a bit late for this to hit -next for me to
> >> feel comfortable about this.
> >
> > OK. I'll revert the patch in my tree.
>
> But it's been in the pipe since end of August and no one reported any
> issue other issue than the pre-existing one, so what's the new issue
> that prevents us to merge it two monthes later, and how do we walk
> forward then ?
We work to resolve the test failure, it's that simple. I haven't seen
the failure so I haven't been much help to do any sort of root cause
digging on the problem, it would be helpful if those who are seeing
the problem could dig into the failure and report back on what they
find. That is what has been missing and why I never ACK'd or merged
the powerpc audit code.
--
paul moore
www.paul-moore.com
^ permalink raw reply
* [PATCH v3] KVM: PPC: Tick accounting should defer vtime accounting 'til after IRQ handling
From: Nicholas Piggin @ 2021-10-27 14:21 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Laurent Vivier, Nicholas Piggin, stable
From: Laurent Vivier <lvivier@redhat.com>
Commit 112665286d08 ("KVM: PPC: Book3S HV: Context tracking exit guest
context before enabling irqs") moved guest_exit() into the interrupt
protected area to avoid wrong context warning (or worse). The problem is
that tick-based time accounting has not yet been updated at this point
(because it depends on the timer interrupt firing), so the guest time
gets incorrectly accounted to system time.
To fix the problem, follow the x86 fix in commit 160457140187 ("Defer
vtime accounting 'til after IRQ handling"), and allow host IRQs to run
before accounting the guest exit time.
In the case vtime accounting is enabled, this is not required because TB
is used directly for accounting.
Before this patch, with CONFIG_TICK_CPU_ACCOUNTING=y in the host and a
guest running a kernel compile, the 'guest' fields of /proc/stat are
stuck at zero. With the patch they can be observed increasing roughly as
expected.
Fixes: e233d54d4d97 ("KVM: booke: use __kvm_guest_exit")
Fixes: 112665286d08 ("KVM: PPC: Book3S HV: Context tracking exit guest context before enabling irqs")
Cc: <stable@vger.kernel.org> # 5.12
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
[np: only required for tick accounting, add Book3E fix, tweak changelog]
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
Since v2:
- I took over the patch with Laurent's blessing.
- Changed to avoid processing IRQs if we do have vtime accounting
enabled.
- Changed so in either case the accounting is called with irqs disabled.
- Added similar Book3E fix.
- Rebased on upstream, tested, observed bug and confirmed fix.
arch/powerpc/kvm/book3s_hv.c | 30 ++++++++++++++++++++++++++++--
arch/powerpc/kvm/booke.c | 16 +++++++++++++++-
2 files changed, 43 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 2acb1c96cfaf..7b74fc0a986b 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -3726,7 +3726,20 @@ static noinline void kvmppc_run_core(struct kvmppc_vcore *vc)
kvmppc_set_host_core(pcpu);
- guest_exit_irqoff();
+ context_tracking_guest_exit();
+ if (!vtime_accounting_enabled_this_cpu()) {
+ local_irq_enable();
+ /*
+ * Service IRQs here before vtime_account_guest_exit() so any
+ * ticks that occurred while running the guest are accounted to
+ * the guest. If vtime accounting is enabled, accounting uses
+ * TB rather than ticks, so it can be done without enabling
+ * interrupts here, which has the problem that it accounts
+ * interrupt processing overhead to the host.
+ */
+ local_irq_disable();
+ }
+ vtime_account_guest_exit();
local_irq_enable();
@@ -4510,7 +4523,20 @@ int kvmhv_run_single_vcpu(struct kvm_vcpu *vcpu, u64 time_limit,
kvmppc_set_host_core(pcpu);
- guest_exit_irqoff();
+ context_tracking_guest_exit();
+ if (!vtime_accounting_enabled_this_cpu()) {
+ local_irq_enable();
+ /*
+ * Service IRQs here before vtime_account_guest_exit() so any
+ * ticks that occurred while running the guest are accounted to
+ * the guest. If vtime accounting is enabled, accounting uses
+ * TB rather than ticks, so it can be done without enabling
+ * interrupts here, which has the problem that it accounts
+ * interrupt processing overhead to the host.
+ */
+ local_irq_disable();
+ }
+ vtime_account_guest_exit();
local_irq_enable();
diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index 977801c83aff..8c15c90dd3a9 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -1042,7 +1042,21 @@ int kvmppc_handle_exit(struct kvm_vcpu *vcpu, unsigned int exit_nr)
}
trace_kvm_exit(exit_nr, vcpu);
- guest_exit_irqoff();
+
+ context_tracking_guest_exit();
+ if (!vtime_accounting_enabled_this_cpu()) {
+ local_irq_enable();
+ /*
+ * Service IRQs here before vtime_account_guest_exit() so any
+ * ticks that occurred while running the guest are accounted to
+ * the guest. If vtime accounting is enabled, accounting uses
+ * TB rather than ticks, so it can be done without enabling
+ * interrupts here, which has the problem that it accounts
+ * interrupt processing overhead to the host.
+ */
+ local_irq_disable();
+ }
+ vtime_account_guest_exit();
local_irq_enable();
--
2.23.0
^ permalink raw reply related
* Re: [PATCH] usb: gadget: Mark USB_FSL_QE broken on 64-bit
From: Li Yang @ 2021-10-27 14:37 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Felipe Balbi, Arnd Bergmann, Greg Kroah-Hartman, linux-usb, lkml,
linuxppc-dev
In-Reply-To: <20211027080849.3276289-1-geert@linux-m68k.org>
On Wed, Oct 27, 2021 at 5:25 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> On 64-bit:
>
> drivers/usb/gadget/udc/fsl_qe_udc.c: In function ‘qe_ep0_rx’:
> drivers/usb/gadget/udc/fsl_qe_udc.c:842:13: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast]
> 842 | vaddr = (u32)phys_to_virt(in_be32(&bd->buf));
> | ^
> In file included from drivers/usb/gadget/udc/fsl_qe_udc.c:41:
> drivers/usb/gadget/udc/fsl_qe_udc.c:843:28: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
> 843 | frame_set_data(pframe, (u8 *)vaddr);
> | ^
>
> The driver assumes physical and virtual addresses are 32-bit, hence it
> cannot work on 64-bit platforms.
The device is truly only used in legacy 32-bit PowerPC chips and never
tested with 64-bit. Thanks.
>
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
Acked-by: Li Yang <leoyang.li@nxp.com>
> ---
> One more casualty of CONFIG_WERROR=y.
> http://kisskb.ellerman.id.au/kisskb/buildresult/14652936/
> ---
> drivers/usb/gadget/udc/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/usb/gadget/udc/Kconfig b/drivers/usb/gadget/udc/Kconfig
> index 8c614bb86c665c77..69394dc1cdfb6436 100644
> --- a/drivers/usb/gadget/udc/Kconfig
> +++ b/drivers/usb/gadget/udc/Kconfig
> @@ -330,6 +330,7 @@ config USB_AMD5536UDC
> config USB_FSL_QE
> tristate "Freescale QE/CPM USB Device Controller"
> depends on FSL_SOC && (QUICC_ENGINE || CPM)
> + depends on !64BIT || BROKEN
> help
> Some of Freescale PowerPC processors have a Full Speed
> QE/CPM2 USB controller, which support device mode with 4
> --
> 2.25.1
>
^ 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