* Re: [PATCH 02/18] powerpc: remove arguments from fault handler functions
From: Nicholas Piggin @ 2020-11-10 8:29 UTC (permalink / raw)
To: Christophe Leroy, linuxppc-dev
In-Reply-To: <6af9a488-3816-9744-db4b-5a3bceb1f0f0@csgroup.eu>
Excerpts from Christophe Leroy's message of November 6, 2020 5:59 pm:
>
>
> Le 05/11/2020 à 15:34, Nicholas Piggin a écrit :
>> Make mm fault handlers all just take the pt_regs * argument and load
>> DAR/DSISR from that. Make those that return a value return long.
>>
>> This is done to make the function signatures match other handlers, which
>> will help with a future patch to add wrappers. Explicit arguments could
>> be added for performance but that would require more wrapper macro
>> variants.
>>
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>> arch/powerpc/include/asm/asm-prototypes.h | 4 ++--
>> arch/powerpc/include/asm/bug.h | 4 ++--
>> arch/powerpc/kernel/exceptions-64e.S | 2 --
>> arch/powerpc/kernel/exceptions-64s.S | 14 ++------------
>> arch/powerpc/kernel/head_40x.S | 10 +++++-----
>> arch/powerpc/kernel/head_8xx.S | 6 +++---
>> arch/powerpc/kernel/head_book3s_32.S | 6 ++----
>> arch/powerpc/kernel/head_booke.h | 4 +---
>> arch/powerpc/mm/book3s64/hash_utils.c | 8 +++++---
>> arch/powerpc/mm/book3s64/slb.c | 11 +++++++----
>> arch/powerpc/mm/fault.c | 16 +++++++++-------
>> 11 files changed, 38 insertions(+), 47 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/asm-prototypes.h b/arch/powerpc/include/asm/asm-prototypes.h
>> index d0b832cbbec8..22c9d08fa3a4 100644
>> --- a/arch/powerpc/include/asm/asm-prototypes.h
>> +++ b/arch/powerpc/include/asm/asm-prototypes.h
>> @@ -82,8 +82,8 @@ void kernel_bad_stack(struct pt_regs *regs);
>> void system_reset_exception(struct pt_regs *regs);
>> void machine_check_exception(struct pt_regs *regs);
>> void emulation_assist_interrupt(struct pt_regs *regs);
>> -long do_slb_fault(struct pt_regs *regs, unsigned long ea);
>> -void do_bad_slb_fault(struct pt_regs *regs, unsigned long ea, long err);
>> +long do_slb_fault(struct pt_regs *regs);
>> +void do_bad_slb_fault(struct pt_regs *regs);
>>
>> /* signals, syscalls and interrupts */
>> long sys_swapcontext(struct ucontext __user *old_ctx,
>> diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h
>> index d714d83bbc7c..2fa0cf6c6011 100644
>> --- a/arch/powerpc/include/asm/bug.h
>> +++ b/arch/powerpc/include/asm/bug.h
>> @@ -111,8 +111,8 @@
>> #ifndef __ASSEMBLY__
>>
>> struct pt_regs;
>> -extern int do_page_fault(struct pt_regs *, unsigned long, unsigned long);
>> -extern int hash__do_page_fault(struct pt_regs *, unsigned long, unsigned long);
>> +extern long do_page_fault(struct pt_regs *);
>> +extern long hash__do_page_fault(struct pt_regs *);
>
> extern is pointless
Thanks. Sorry I'll get it right one day.
>> @@ -191,9 +191,9 @@ _ENTRY(saved_ksp_limit)
>> */
>> START_EXCEPTION(0x0400, InstructionAccess)
>> EXCEPTION_PROLOG
>> - mr r4,r12 /* Pass SRR0 as arg2 */
>> - stw r4, _DEAR(r11)
>> - li r5,0 /* Pass zero as arg3 */
>> + li r5,0
>> + stw r5, _ESR(r11) /* Zero ESR */
>> + stw r12, _DEAR(r11) /* SRR0 as DEAR */
>
> I think we should avoid this, see below
>
>> @@ -356,14 +356,14 @@ DataStoreTLBMiss:
>> . = 0x1300
>> InstructionTLBError:
>> EXCEPTION_PROLOG
>> - mr r4,r12
>> andis. r5,r9,DSISR_SRR1_MATCH_32S@h /* Filter relevant SRR1 bits */
>
> Could avoid this, see below
>
>> andis. r10,r9,SRR1_ISI_NOPT@h
>> beq+ .Litlbie
>> - tlbie r4
>> + tlbie r12
>> /* 0x400 is InstructionAccess exception, needed by bad_page_fault() */
>> .Litlbie:
>> - stw r4, _DAR(r11)
>> + stw r12, _DAR(r11)
>> + stw r5, _DSISR(r11)
>
> And this
>> @@ -369,9 +369,9 @@ BEGIN_MMU_FTR_SECTION
>> bl hash_page
>> END_MMU_FTR_SECTION_IFSET(MMU_FTR_HPTE_TABLE)
>> #endif /* CONFIG_VMAP_STACK */
>> -1: mr r4,r12
>> andis. r5,r9,DSISR_SRR1_MATCH_32S@h /* Filter relevant SRR1 bits */
>> - stw r4, _DAR(r11)
>> + stw r5, _DSISR(r11)
>> + stw r12, _DAR(r11)
>
> And this including the andis.
>
>> @@ -477,9 +477,7 @@ ALT_FTR_SECTION_END_IFSET(CPU_FTR_EMB_HV)
>> NORMAL_EXCEPTION_PROLOG(INST_STORAGE); \
>> mfspr r5,SPRN_ESR; /* Grab the ESR and save it */ \
>> stw r5,_ESR(r11); \
>> - mr r4,r12; /* Pass SRR0 as arg2 */ \
>> - stw r4, _DEAR(r11); \
>> - li r5,0; /* Pass zero as arg3 */ \
>> + stw r12, _DEAR(r11); /* Pass SRR0 as arg2 */ \
>
> And this
>
[...]
>> diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
>> index e65a49f246ef..390a296b16a3 100644
>> --- a/arch/powerpc/mm/fault.c
>> +++ b/arch/powerpc/mm/fault.c
>> @@ -549,11 +549,12 @@ static int __do_page_fault(struct pt_regs *regs, unsigned long address,
>> }
>> NOKPROBE_SYMBOL(__do_page_fault);
>>
>> -int do_page_fault(struct pt_regs *regs, unsigned long address,
>> - unsigned long error_code)
>> +long do_page_fault(struct pt_regs *regs)
>> {
>> enum ctx_state prev_state = exception_enter();
>> - int err;
>> + unsigned long address = regs->dar;
>> + unsigned long error_code = regs->dsisr;
>> + long err;
>
> By doing something more or less like this (need to be tuned for bookE as well):
>
> + int is_exec = TRAP(regs) == 0x400;
> + unsigned long address = is_exec ? regs->ssr0 : regs->dar;
> + unsigned long error_code = is_exec ? (regs->ssr1 & DSISR_SRR1_MATCH_32S) : regs->dsisr;
Ah, I didn't see that you saved these in srr0/1 already. Hmm, not in
pt_regs though. thread_struct (VMAP_STACK only)? exception_regs (booke
only)? Doesn't seem so easy.
In general I don't have a problem with some processor specific things
like this in do page_fault though if it speeds things up. If it gets
a bit more complicated then we can have some accsssor function
get_fault_details(regs, &address, &error_code, &is_exec);
>> @@ -580,11 +581,12 @@ int do_page_fault(struct pt_regs *regs, unsigned long address,
>> NOKPROBE_SYMBOL(do_page_fault);
>>
>> #ifdef CONFIG_PPC_BOOK3S_64
>> -/* Same as do_page_fault but interrupt entry has already run in do_hash_fault */
>> -int hash__do_page_fault(struct pt_regs *regs, unsigned long address,
>> - unsigned long error_code)
>> +/* Same as do_page_fault but no interrupt entry */
>> +long hash__do_page_fault(struct pt_regs *regs)
>> {
>> - int err;
>> + unsigned long address = regs->dar;
>> + unsigned long error_code = regs->dsisr;
>> + long err;
>>
>> err = __do_page_fault(regs, address, error_code);
>> if (unlikely(err)) {
>>
>
> There is probably also something we can simplify around get_and_save_dar_dsisr_on_stack() macro in
> head_32.h, no need to reload DAR, at least for 8xx. Maybe as a followup patch later.
Admittedly my 32-bit knowledge or test environments are not great :(
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH 03/18] powerpc: bad_page_fault, do_break get registers from regs
From: Nicholas Piggin @ 2020-11-10 8:34 UTC (permalink / raw)
To: Christophe Leroy, linuxppc-dev
In-Reply-To: <8b325832-b843-7d01-8b0f-fc278c444ce5@csgroup.eu>
Excerpts from Christophe Leroy's message of November 6, 2020 6:14 pm:
>
>
> Le 05/11/2020 à 15:34, Nicholas Piggin a écrit :
>> This also moves the 32s DABR match to C.
>
> Is there a real benefit doing this ?
Oh I missed doing it, but yes I think bad_page_fault and do_break should
probably be implemented with the DEFINE_INTERRUT_HANDLER wrappers.
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH 18/18] powerpc/64s: move power4 idle entirely to C
From: Nicholas Piggin @ 2020-11-10 8:43 UTC (permalink / raw)
To: Christophe Leroy, linuxppc-dev
In-Reply-To: <7de6fd21-da79-fe8f-5db4-f99ee0dd7d23@csgroup.eu>
Excerpts from Christophe Leroy's message of November 7, 2020 7:43 pm:
>
>
> Le 05/11/2020 à 15:34, Nicholas Piggin a écrit :
>> Christophe asked about doing this, most of the code is still in
>> asm but maybe it's slightly nicer? I don't know if it's worthwhile.
>
> Heu... I don't think I was asking for that, but why not, see later comments.
>
> At first I was just asking to write the following in C:
>
> +
> + .globl power4_idle_nap_return
> +power4_idle_nap_return:
> + blr
>
>
> In extenso, instead of the above do somewhere something like:
>
> void power4_idle_nap_return(void)
> {
> }
Ah! Well either was a good question. I don't mind attempting it :)
>> ---
>> arch/powerpc/kernel/idle.c | 25 ++++++++++++++++++++-----
>> arch/powerpc/kernel/idle_book3s.S | 22 ----------------------
>> 2 files changed, 20 insertions(+), 27 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/idle.c b/arch/powerpc/kernel/idle.c
>> index ae0e2632393d..849e77a45915 100644
>> --- a/arch/powerpc/kernel/idle.c
>> +++ b/arch/powerpc/kernel/idle.c
>> @@ -72,6 +72,9 @@ int powersave_nap;
>> #ifdef CONFIG_PPC_970_NAP
>> void power4_idle(void)
>> {
>> + unsigned long msr_idle = MSR_KERNEL|MSR_EE|MSR_POW;
>> + unsigned long tmp1, tmp2;
>> +
>> if (!cpu_has_feature(CPU_FTR_CAN_NAP))
>> return;
>>
>> @@ -84,13 +87,25 @@ void power4_idle(void)
>> if (cpu_has_feature(CPU_FTR_ALTIVEC))
>> asm volatile("DSSALL ; sync" ::: "memory");
>>
>> - power4_idle_nap();
>> -
>> + asm volatile(
>> +" ld %0,PACA_THREAD_INFO(r13) \n"
>> +" ld %1,TI_LOCAL_FLAGS(%0) \n"
>> +" ori %1,%1,_TLF_NAPPING \n"
>> +" std %1,TI_LOCAL_FLAGS(%0) \n"
>
> Can't this just be:
>
> current_thread_info()->local_flags |= _TLF_NAPPING;
>
>> /*
>> - * power4_idle_nap returns with interrupts enabled (soft and hard).
>> - * to our caller with interrupts enabled (soft and hard). Our caller
>> - * can cope with either interrupts disabled or enabled upon return.
>> + * NAPPING bit is set, from this point onward nap_adjust_return()
>> + * will cause interrupts to return to power4_idle_nap_return.
>> */
>> +"1: sync \n"
>> +" isync \n"
>> +" mtmsrd %2 \n"
>> +" isync \n"
>> +" b 1b \n"
>
> And this:
>
> for (;;) {
> mb();
> isync();
> mtmsr(MSR_KERNEL|MSR_EE|MSR_POW);
> isync();
> }
I was hoping something nicer like this but I think not because as soon
as we set _TLF_NAPPING, we might take an interrupt which returns
somewhere else, and you aren't allowed to do that in C code (mainly
because the stack and register state would be unknown). Even going
immediately to blr or end of function might miss restoring NVGPRs etc.
There might be some tricks we could play with soft-masking interrupts,
using MSR[EE]=0, and then doing all this and returning to right after
the mtmsr POW with a flag set... But it's a bit of tricky churn for an
old CPU that works okay.
Thanks,
Nick
>
>
>> +" .globl power4_idle_nap_return \n"
>> +"power4_idle_nap_return: \n"
>> + : "=r"(tmp1), "=r"(tmp2)
>> + : "r"(msr_idle)
>> + );
>> }
>> #endif
>>
>
> Christophe
>
^ permalink raw reply
* Re: [PATCH v2 1/3] powerpc/64s: Replace RFI by RFI_TO_KERNEL and remove RFI
From: Nicholas Piggin @ 2020-11-10 8:45 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Christophe Leroy, Michael Ellerman,
Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <7719261b0a0d2787772339484c33eb809723bca7.1604854583.git.christophe.leroy@csgroup.eu>
Excerpts from Christophe Leroy's message of November 9, 2020 2:57 am:
> In head_64.S, we have two places using RFI to return to
> kernel. Use RFI_TO_KERNEL instead.
>
> They are the two only places using RFI on book3s/64, so
> the RFI macro can go away.
Looks good to me.
Acked-by: Nicholas Piggin <npiggin@gmail.com>
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> arch/powerpc/include/asm/ppc_asm.h | 1 -
> arch/powerpc/kernel/head_64.S | 9 +++++++--
> 2 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/ppc_asm.h b/arch/powerpc/include/asm/ppc_asm.h
> index 511786f0e40d..bedf3eb52ebc 100644
> --- a/arch/powerpc/include/asm/ppc_asm.h
> +++ b/arch/powerpc/include/asm/ppc_asm.h
> @@ -495,7 +495,6 @@ END_FTR_SECTION_NESTED(CPU_FTR_CELL_TB_BUG, CPU_FTR_CELL_TB_BUG, 96)
> #endif
>
> #ifdef CONFIG_PPC_BOOK3S_64
> -#define RFI rfid
> #define MTMSRD(r) mtmsrd r
> #define MTMSR_EERI(reg) mtmsrd reg,1
> #else
> diff --git a/arch/powerpc/kernel/head_64.S b/arch/powerpc/kernel/head_64.S
> index 1510b2a56669..ecf9a88988ff 100644
> --- a/arch/powerpc/kernel/head_64.S
> +++ b/arch/powerpc/kernel/head_64.S
> @@ -41,6 +41,11 @@
> #include <asm/ppc-opcode.h>
> #include <asm/export.h>
> #include <asm/feature-fixups.h>
> +#ifdef CONFIG_PPC_BOOK3S
> +#include <asm/exception-64s.h>
> +#else
> +#include <asm/exception-64e.h>
> +#endif
>
> /* The physical memory is laid out such that the secondary processor
> * spin code sits at 0x0000...0x00ff. On server, the vectors follow
> @@ -829,7 +834,7 @@ __secondary_start:
>
> mtspr SPRN_SRR0,r3
> mtspr SPRN_SRR1,r4
> - RFI
> + RFI_TO_KERNEL
> b . /* prevent speculative execution */
>
> /*
> @@ -966,7 +971,7 @@ start_here_multiplatform:
> ld r4,PACAKMSR(r13)
> mtspr SPRN_SRR0,r3
> mtspr SPRN_SRR1,r4
> - RFI
> + RFI_TO_KERNEL
> b . /* prevent speculative execution */
>
> /* This is where all platforms converge execution */
> --
> 2.25.0
>
>
^ permalink raw reply
* Re: [PATCH RFC PKS/PMEM 05/58] kmap: Introduce k[un]map_thread
From: Thomas Gleixner @ 2020-11-10 8:48 UTC (permalink / raw)
To: Ira Weiny
Cc: linux-aio, linux-efi, kvm, linux-doc, Peter Zijlstra, linux-mmc,
Dave Hansen, dri-devel, linux-mm, target-devel, linux-mtd,
linux-kselftest, samba-technical, ceph-devel, drbd-dev, devel,
linux-cifs, linux-nilfs, linux-scsi, linux-nvdimm, linux-rdma,
x86, amd-gfx, io-uring, cluster-devel, Ingo Molnar,
intel-wired-lan, xen-devel, linux-ext4, Fenghua Yu, linux-afs,
linux-um, intel-gfx, ecryptfs, linux-erofs, reiserfs-devel,
linux-block, linux-bcache, Borislav Petkov, Andy Lutomirski,
Dan Williams, Andrew Morton, linux-cachefs, linux-nfs,
linux-ntfs-dev, netdev, Randy Dunlap, kexec, linux-kernel,
linux-f2fs-devel, linux-fsdevel, bpf, linuxppc-dev, linux-btrfs
In-Reply-To: <20201110045954.GL3976735@iweiny-DESK2.sc.intel.com>
On Mon, Nov 09 2020 at 20:59, Ira Weiny wrote:
> On Tue, Nov 10, 2020 at 02:13:56AM +0100, Thomas Gleixner wrote:
> Also, we can convert the new memcpy_*_page() calls to kmap_local() as well.
> [For now my patch just uses kmap_atomic().]
>
> I've not looked at all of the patches in your latest version. Have you
> included converting any of the kmap() call sites? I thought you were more
> focused on converting the kmap_atomic() to kmap_local()?
I did not touch any of those yet, but it's a logical consequence to
convert all kmap() instances which are _not_ creating a global mapping
over to it.
Thanks,
tglx
^ permalink raw reply
* Re: [RFC PATCH 0/9] powerpc/64s: fast interrupt exit
From: Nicholas Piggin @ 2020-11-10 8:49 UTC (permalink / raw)
To: Christophe Leroy, linuxppc-dev
In-Reply-To: <fdde16b8-2bb4-f6a2-3c29-61d0169453cf@csgroup.eu>
Excerpts from Christophe Leroy's message of November 7, 2020 8:35 pm:
>
>
> Le 06/11/2020 à 16:59, Nicholas Piggin a écrit :
>> This series attempts to improve the speed of interrupts and system calls
>> in two major ways.
>>
>> Firstly, the SRR/HSRR registers do not need to be reloaded if they were
>> not used or clobbered fur the duration of the interrupt.
>>
>> Secondly, an alternate return location facility is added for soft-masked
>> asynchronous interrupts and then that's used to set everything up for
>> return without having to disable MSR RI or EE.
>>
>> After this series, the entire system call / interrupt handler fast path
>> executes no mtsprs and one mtmsrd to enable interrupts initially, and
>> the system call vectored path doesn't even need to do that.
>
> Interesting series.
>
> Unfortunately, can't be done on PPC32 (at least on non bookE), because it would mean mapping kernel
> at 0 instead of 0xC0000000. Not sure libc would like it, and anyway it would be an issue for
> catching NULL pointer dereferencing, unless we use page tables instead of BATs to map kernel mem,
> which would be serious performance cut.
Hmm, why would you have to map at 0?
PPC32 doesn't have soft mask interrupts, but you could still test all
MSR[PR]=0 interrupts to see if they land inside some region to see if
they hit in the restart table I think?
Could PPC32 skip the SRR reload at least? That's simpler.
Thanks,
Nick
^ permalink raw reply
* [PATCH V3] sched/rt, powerpc: Prepare for PREEMPT_RT
From: Wang Qing @ 2020-11-10 8:53 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
Christophe Leroy, Nicholas Piggin, Jordan Niethe, Alistair Popple,
Wang Qing, Aneesh Kumar K.V, Peter Zijlstra, Greg Kroah-Hartman,
linuxppc-dev, linux-kernel
PREEMPT_RT is a separate preemption model, CONFIG_PREEMPT will
be disabled when CONFIG_PREEMPT_RT is enabled, so we need
to add CONFIG_PREEMPT_RT output to __die().
Signed-off-by: Wang Qing <wangqing@vivo.com>
Changes in v3:
- Fix typo issue.
Changes in v2:
- Modify as Christophe suggested.
---
arch/powerpc/kernel/traps.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index 5006dcb..dec7b81
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -262,10 +262,11 @@ static int __die(const char *str, struct pt_regs *regs, long err)
{
printk("Oops: %s, sig: %ld [#%d]\n", str, err, ++die_counter);
- printk("%s PAGE_SIZE=%luK%s%s%s%s%s%s %s\n",
+ printk("%s PAGE_SIZE=%luK%s%s%s%s%s%s%s %s\n",
IS_ENABLED(CONFIG_CPU_LITTLE_ENDIAN) ? "LE" : "BE",
PAGE_SIZE / 1024, get_mmu_str(),
IS_ENABLED(CONFIG_PREEMPT) ? " PREEMPT" : "",
+ IS_ENABLED(CONFIG_PREEMPT_RT) ? " PREEMPT_RT" : "",
IS_ENABLED(CONFIG_SMP) ? " SMP" : "",
IS_ENABLED(CONFIG_SMP) ? (" NR_CPUS=" __stringify(NR_CPUS)) : "",
debug_pagealloc_enabled() ? " DEBUG_PAGEALLOC" : "",
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v7 4/4] arch, mm: make kernel_page_present() always available
From: David Hildenbrand @ 2020-11-10 10:02 UTC (permalink / raw)
To: Mike Rapoport, Andrew Morton
Cc: Peter Zijlstra, Dave Hansen, linux-mm, Paul Mackerras,
Pavel Machek, H. Peter Anvin, sparclinux, Christoph Lameter,
Will Deacon, linux-riscv, linux-s390, x86, Mike Rapoport,
Christian Borntraeger, Ingo Molnar, linux-arm-kernel,
Catalin Marinas, Len Brown, Albert Ou, Vasily Gorbik, linux-pm,
Heiko Carstens, David Rientjes, Borislav Petkov, Andy Lutomirski,
Paul Walmsley, Kirill A. Shutemov, Thomas Gleixner,
Vlastimil Babka, Rafael J. Wysocki, linux-kernel, Pekka Enberg,
Palmer Dabbelt, Joonsoo Kim, Edgecombe, Rick P, linuxppc-dev,
David S. Miller, Kirill A . Shutemov
In-Reply-To: <20201109192128.960-5-rppt@kernel.org>
On 09.11.20 20:21, Mike Rapoport wrote:
> From: Mike Rapoport <rppt@linux.ibm.com>
>
> For architectures that enable ARCH_HAS_SET_MEMORY having the ability to
> verify that a page is mapped in the kernel direct map can be useful
> regardless of hibernation.
>
> Add RISC-V implementation of kernel_page_present(), update its forward
> declarations and stubs to be a part of set_memory API and remove ugly
> ifdefery in inlcude/linux/mm.h around current declarations of
> kernel_page_present().
>
> Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
> Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Acked-by: David Hildenbrand <david@redhat.com>
--
Thanks,
David / dhildenb
^ permalink raw reply
* Re: [PATCH V3] sched/rt, powerpc: Prepare for PREEMPT_RT
From: Christophe Leroy @ 2020-11-10 10:57 UTC (permalink / raw)
To: Wang Qing, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Nicholas Piggin, Jordan Niethe, Alistair Popple,
Aneesh Kumar K.V, Peter Zijlstra, Greg Kroah-Hartman,
linuxppc-dev, linux-kernel
In-Reply-To: <1604998411-16116-1-git-send-email-wangqing@vivo.com>
Le 10/11/2020 à 09:53, Wang Qing a écrit :
> PREEMPT_RT is a separate preemption model, CONFIG_PREEMPT will
> be disabled when CONFIG_PREEMPT_RT is enabled, so we need
> to add CONFIG_PREEMPT_RT output to __die().
>
> Signed-off-by: Wang Qing <wangqing@vivo.com>
Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>
> Changes in v3:
> - Fix typo issue.
>
> Changes in v2:
> - Modify as Christophe suggested.
> ---
> arch/powerpc/kernel/traps.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> index 5006dcb..dec7b81
> --- a/arch/powerpc/kernel/traps.c
> +++ b/arch/powerpc/kernel/traps.c
> @@ -262,10 +262,11 @@ static int __die(const char *str, struct pt_regs *regs, long err)
> {
> printk("Oops: %s, sig: %ld [#%d]\n", str, err, ++die_counter);
>
> - printk("%s PAGE_SIZE=%luK%s%s%s%s%s%s %s\n",
> + printk("%s PAGE_SIZE=%luK%s%s%s%s%s%s%s %s\n",
> IS_ENABLED(CONFIG_CPU_LITTLE_ENDIAN) ? "LE" : "BE",
> PAGE_SIZE / 1024, get_mmu_str(),
> IS_ENABLED(CONFIG_PREEMPT) ? " PREEMPT" : "",
> + IS_ENABLED(CONFIG_PREEMPT_RT) ? " PREEMPT_RT" : "",
> IS_ENABLED(CONFIG_SMP) ? " SMP" : "",
> IS_ENABLED(CONFIG_SMP) ? (" NR_CPUS=" __stringify(NR_CPUS)) : "",
> debug_pagealloc_enabled() ? " DEBUG_PAGEALLOC" : "",
>
^ permalink raw reply
* Re: [PATCH 02/18] powerpc: remove arguments from fault handler functions
From: Christophe Leroy @ 2020-11-10 11:15 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev
In-Reply-To: <1604996406.ltcjkqarcr.astroid@bobo.none>
Le 10/11/2020 à 09:29, Nicholas Piggin a écrit :
> Excerpts from Christophe Leroy's message of November 6, 2020 5:59 pm:
>>
>>
>> Le 05/11/2020 à 15:34, Nicholas Piggin a écrit :
>>> Make mm fault handlers all just take the pt_regs * argument and load
>>> DAR/DSISR from that. Make those that return a value return long.
>>>
>>> This is done to make the function signatures match other handlers, which
>>> will help with a future patch to add wrappers. Explicit arguments could
>>> be added for performance but that would require more wrapper macro
>>> variants.
>>>
>>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>>> ---
[...]
>
>>> diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
>>> index e65a49f246ef..390a296b16a3 100644
>>> --- a/arch/powerpc/mm/fault.c
>>> +++ b/arch/powerpc/mm/fault.c
>>> @@ -549,11 +549,12 @@ static int __do_page_fault(struct pt_regs *regs, unsigned long address,
>>> }
>>> NOKPROBE_SYMBOL(__do_page_fault);
>>>
>>> -int do_page_fault(struct pt_regs *regs, unsigned long address,
>>> - unsigned long error_code)
>>> +long do_page_fault(struct pt_regs *regs)
>>> {
>>> enum ctx_state prev_state = exception_enter();
>>> - int err;
>>> + unsigned long address = regs->dar;
>>> + unsigned long error_code = regs->dsisr;
>>> + long err;
>>
>> By doing something more or less like this (need to be tuned for bookE as well):
>>
>> + int is_exec = TRAP(regs) == 0x400;
>> + unsigned long address = is_exec ? regs->ssr0 : regs->dar;
>> + unsigned long error_code = is_exec ? (regs->ssr1 & DSISR_SRR1_MATCH_32S) : regs->dsisr;
>
> Ah, I didn't see that you saved these in srr0/1 already. Hmm, not in
> pt_regs though. thread_struct (VMAP_STACK only)? exception_regs (booke
> only)? Doesn't seem so easy.
Oops yes you are right, SRR0/SRR1 are not in pt_regs. And their validity in thread struct is rather
short ... So forget my comment.
Christophe
^ permalink raw reply
* Re: [PATCH 03/18] powerpc: bad_page_fault, do_break get registers from regs
From: Christophe Leroy @ 2020-11-10 11:19 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev
In-Reply-To: <1604996998.52nfki5192.astroid@bobo.none>
Le 10/11/2020 à 09:34, Nicholas Piggin a écrit :
> Excerpts from Christophe Leroy's message of November 6, 2020 6:14 pm:
>>
>>
>> Le 05/11/2020 à 15:34, Nicholas Piggin a écrit :
>>> This also moves the 32s DABR match to C.
>>
>> Is there a real benefit doing this ?
>
> Oh I missed doing it, but yes I think bad_page_fault and do_break should
> probably be implemented with the DEFINE_INTERRUT_HANDLER wrappers.
>
Yes, anyway, do we need to do that change ? Can't the dispatch between do_break() and page fault
handling remain in handle_page_fault() ? What's the benefit of going into do_page_fault() and coming
back ?
Christophe
^ permalink raw reply
* [PATCH] powerpc/powernv/sriov: fix unsigned int win compared to less than zero
From: xiakaixu1987 @ 2020-11-10 11:19 UTC (permalink / raw)
To: fbarrat, ajd, mpe, benh, paulus; +Cc: Kaixu Xia, linuxppc-dev, linux-kernel
From: Kaixu Xia <kaixuxia@tencent.com>
Fix coccicheck warning:
./arch/powerpc/platforms/powernv/pci-sriov.c:443:7-10: WARNING: Unsigned expression compared with zero: win < 0
./arch/powerpc/platforms/powernv/pci-sriov.c:462:7-10: WARNING: Unsigned expression compared with zero: win < 0
Reported-by: Tosk Robot <tencent_os_robot@tencent.com>
Signed-off-by: Kaixu Xia <kaixuxia@tencent.com>
---
arch/powerpc/platforms/powernv/pci-sriov.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/platforms/powernv/pci-sriov.c b/arch/powerpc/platforms/powernv/pci-sriov.c
index c4434f20f42f..92fc861c528f 100644
--- a/arch/powerpc/platforms/powernv/pci-sriov.c
+++ b/arch/powerpc/platforms/powernv/pci-sriov.c
@@ -422,7 +422,7 @@ static int pnv_pci_vf_assign_m64(struct pci_dev *pdev, u16 num_vfs)
{
struct pnv_iov_data *iov;
struct pnv_phb *phb;
- unsigned int win;
+ int win;
struct resource *res;
int i, j;
int64_t rc;
--
2.20.0
^ permalink raw reply related
* Re: [RFC PATCH 0/9] powerpc/64s: fast interrupt exit
From: Christophe Leroy @ 2020-11-10 11:31 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev
In-Reply-To: <1604997971.w6spl33ij0.astroid@bobo.none>
Le 10/11/2020 à 09:49, Nicholas Piggin a écrit :
> Excerpts from Christophe Leroy's message of November 7, 2020 8:35 pm:
>>
>>
>> Le 06/11/2020 à 16:59, Nicholas Piggin a écrit :
>>> This series attempts to improve the speed of interrupts and system calls
>>> in two major ways.
>>>
>>> Firstly, the SRR/HSRR registers do not need to be reloaded if they were
>>> not used or clobbered fur the duration of the interrupt.
>>>
>>> Secondly, an alternate return location facility is added for soft-masked
>>> asynchronous interrupts and then that's used to set everything up for
>>> return without having to disable MSR RI or EE.
>>>
>>> After this series, the entire system call / interrupt handler fast path
>>> executes no mtsprs and one mtmsrd to enable interrupts initially, and
>>> the system call vectored path doesn't even need to do that.
>>
>> Interesting series.
>>
>> Unfortunately, can't be done on PPC32 (at least on non bookE), because it would mean mapping kernel
>> at 0 instead of 0xC0000000. Not sure libc would like it, and anyway it would be an issue for
>> catching NULL pointer dereferencing, unless we use page tables instead of BATs to map kernel mem,
>> which would be serious performance cut.
>
> Hmm, why would you have to map at 0?
In real mode, physical mem is at 0. If we want to switch from real to virtual mode by just writing
to MSR, then we need virtuel addresses match with real addresses ?
We could play with chip selects to put RAM at 0xc0000000, but then we'd have a problem with
exception as they have to be at 0. Or we could play with MSR[IP] and get the exceptions at
0xfff00000, but that would not be so easy I guess.
>
> PPC32 doesn't have soft mask interrupts, but you could still test all
> MSR[PR]=0 interrupts to see if they land inside some region to see if
> they hit in the restart table I think?
Yes and this is already what is done at exit for the ones that handle MSR[RI] I think.
>
> Could PPC32 skip the SRR reload at least? That's simpler.
I think that would only be possible if real addresses where matching virtual addresses, or am I
missing something ?
Christophe
^ permalink raw reply
* Re: [PATCH] powerpc/powernv/sriov: fix unsigned int win compared to less than zero
From: Andrew Donnellan @ 2020-11-10 12:01 UTC (permalink / raw)
To: xiakaixu1987, fbarrat, mpe, benh, paulus
Cc: Kaixu Xia, linuxppc-dev, linux-kernel
In-Reply-To: <1605007170-22171-1-git-send-email-kaixuxia@tencent.com>
On 10/11/20 10:19 pm, xiakaixu1987@gmail.com wrote:
> From: Kaixu Xia <kaixuxia@tencent.com>
>
> Fix coccicheck warning:
>
> ./arch/powerpc/platforms/powernv/pci-sriov.c:443:7-10: WARNING: Unsigned expression compared with zero: win < 0
> ./arch/powerpc/platforms/powernv/pci-sriov.c:462:7-10: WARNING: Unsigned expression compared with zero: win < 0
>
> Reported-by: Tosk Robot <tencent_os_robot@tencent.com>
> Signed-off-by: Kaixu Xia <kaixuxia@tencent.com>
This seems like the right fix, the value assigned to win can indeed be
-1 so it should be signed. Thanks for sending the patch.
Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com>
> ---
> arch/powerpc/platforms/powernv/pci-sriov.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/platforms/powernv/pci-sriov.c b/arch/powerpc/platforms/powernv/pci-sriov.c
> index c4434f20f42f..92fc861c528f 100644
> --- a/arch/powerpc/platforms/powernv/pci-sriov.c
> +++ b/arch/powerpc/platforms/powernv/pci-sriov.c
> @@ -422,7 +422,7 @@ static int pnv_pci_vf_assign_m64(struct pci_dev *pdev, u16 num_vfs)
> {
> struct pnv_iov_data *iov;
> struct pnv_phb *phb;
> - unsigned int win;
> + int win;
> struct resource *res;
> int i, j;
> int64_t rc;
>
--
Andrew Donnellan OzLabs, ADL Canberra
ajd@linux.ibm.com IBM Australia Limited
^ permalink raw reply
* Re: [PATCH] powerpc/pseries/hotplug-cpu: Fix memleak when cpus node not exist
From: Nathan Lynch @ 2020-11-10 14:08 UTC (permalink / raw)
To: Zhang Xiaoxu; +Cc: tyreld, groug, paulus, linuxppc-dev
In-Reply-To: <20201110123029.3767459-1-zhangxiaoxu5@huawei.com>
Zhang Xiaoxu <zhangxiaoxu5@huawei.com> writes:
> From: zhangxiaoxu <zhangxiaoxu5@huawei.com>
>
> If the cpus nodes not exist, we lost to free the 'cpu_drcs', which
> will leak memory.
>
> Fixes: a0ff72f9f5a7 ("powerpc/pseries/hotplug-cpu: Remove double free in error path")
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: zhangxiaoxu <zhangxiaoxu5@huawei.com>
> ---
> arch/powerpc/platforms/pseries/hotplug-cpu.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
> index f2837e33bf5d..4bb1c9f2bb11 100644
> --- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
> +++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
> @@ -743,6 +743,7 @@ static int dlpar_cpu_add_by_count(u32 cpus_to_add)
> parent = of_find_node_by_path("/cpus");
> if (!parent) {
> pr_warn("Could not find CPU root node in device tree\n");
> + kfree(cpu_drcs);
> return -1;
> }
Thanks for finding this.
a0ff72f9f5a7 ("powerpc/pseries/hotplug-cpu: Remove double free in error
path") was posted in Sept 2019 but was not applied until July 2020:
https://lore.kernel.org/linuxppc-dev/20190919231633.1344-1-nathanl@linux.ibm.com/
Here is that change as posted; note the function context is
find_dlpar_cpus_to_add(), not dlpar_cpu_add_by_count():
--- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
+++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
@@ -726,7 +726,6 @@ static int find_dlpar_cpus_to_add(u32 *cpu_drcs, u32 cpus_to_add)
parent = of_find_node_by_path("/cpus");
if (!parent) {
pr_warn("Could not find CPU root node in device tree\n");
- kfree(cpu_drcs);
return -1;
}
Meanwhile b015f6bc9547dbc056edde7177c7868ca8629c4c ("powerpc/pseries: Add
cpu DLPAR support for drc-info property") was posted in Nov 2019 and
committed a few days later:
https://lore.kernel.org/linux-pci/1573449697-5448-4-git-send-email-tyreld@linux.ibm.com/
This change reorganized the same code, removing
find_dlpar_cpus_to_add(), and it had the effect of fixing the same
issue.
However git apparently allowed the older change to still apply on top of
this (changing a function different from the one in the original
patch!), leading to a real bug.
Your patch is correct but it should be framed as a revert of
a0ff72f9f5a7 with this context in the commit message.
^ permalink raw reply
* Re: [PATCH v3 01/16] dt-bindings: usb: usb-hcd: Convert generic USB properties to DT schema
From: Serge Semin @ 2020-11-10 15:05 UTC (permalink / raw)
To: Chunfeng Yun, Rob Herring, Greg Kroah-Hartman, Rob Herring
Cc: Neil Armstrong, linux-kernel, Pavel Parkhomenko, Kevin Hilman,
Krzysztof Kozlowski, Andy Gross, linux-snps-arc, devicetree,
Mathias Nyman, Martin Blumenstingl, Lad Prabhakar, Alexey Malahov,
Bjorn Andersson, linux-arm-kernel, Roger Quadros, Felipe Balbi,
Yoshihiro Shimoda, linux-usb, linux-mips, Serge Semin,
Manu Gautam, linuxppc-dev
In-Reply-To: <20201026164648.jmp6uiblwoxrqehb@mobilestation>
It seems noone is going to get involved in the discussion. Therefore I'll
fix the patch in the way I suggested in my previous message. Alas I'll
have to remove the reviewed-by tags of Rob from some patches.
-Sergey
On Mon, Oct 26, 2020 at 07:46:49PM +0300, Serge Semin wrote:
> Folks, any comment on my previous message below?
>
> On Wed, Oct 21, 2020 at 06:46:21PM +0300, Serge Semin wrote:
> > On Wed, Oct 21, 2020 at 11:00:36AM +0800, Chunfeng Yun wrote:
> > > On Tue, 2020-10-20 at 14:20 +0300, Serge Semin wrote:
> > > > The generic USB HCD properties have been described in the legacy bindings
> > > > text file: Documentation/devicetree/bindings/usb/generic.txt . Let's
> > > > convert it' content into the USB HCD DT schema properties so all USB DT
> > > ^ its?
> > > > nodes would be validated to have them properly utilized.
> > > >
> > > > Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
> > > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > >
> > > > ---
> > > >
> > > > Changelog v2:
> > > > - Discard '|' in all the new properties, since we don't need to preserve
> > > > the text formatting.
> > > > - Convert abbreviated form of the "maximum-speed" enum restriction into
> > > > the multi-lined version of the list.
> > > > - Drop quotes from around the string constants.
> > > > ---
> > > > .../devicetree/bindings/usb/generic.txt | 57 ------------
> > > > .../devicetree/bindings/usb/usb-hcd.yaml | 88 +++++++++++++++++++
> >
> > > Do we need change the file name or modify it's title?
> > > the title is "Generic USB Host Controller Device Tree Bindings", but
> > > some generic properties, such as, dr_mode, usb-role-switch, otg related
> > > ones, are usually used by DRD controller, this may cause some confusion.
> >
> > Hm, good question. A problem of the naming and the properties inclusion do
> > exist here. I haven't thought that through when moved all the generic
> > properties into the usb-hcd.yaml bindings file. But I don't think it's a good
> > idea to rename the file. Most likely the best solution would be to split the
> > functionality up as follows:
> >
> > 1) usb.yaml - common USB controller with generic properties for all USB modes (host, peripheral, otg):
> > + $nodename
> > + phys
> > + phy-names
> > + usb-phy
> > + maximum-speed
> > + phy_type
> > 2) usb-hcd.yaml - DT schema for USB host controllers (EHCI/OHCI/UHCI):
> > + allOf: [usb.yaml#]
> > + companion
> > 3) usb-xhci.yaml - DT schema for USB host controllers (xHCI):
> > + allOf: [usb-hcd.yaml#]
> > + usb2-lpm-disable
> > + usb3-lpm-capable
> > + quirk-broken-port-ped
> > + imod-interval-ns
> > 4) usb-drd.yaml - DT schema for USB OTG controllers:
> > + otg-rev
> > + hnp-disable
> > + srp-disable
> > + adp-disable
> > + usb-role-switch
> > + role-switch-default-mode
> > + tpl-support
> > + dr_mode: [host, peripheral, otg]
> >
> > So in case if an USB controller is DRD with EHCI host, then it will need
> > to pass evaluation of allOf: [usb-hcd.yaml#, usb-drd.yaml#]. If an USB
> > controller is DRD with xHCI host, then the next schema can be applied:
> > [usb-xhci.yaml#, usb-drd.yaml#]. A conditional schema is also applicable
> > here, like this:
> > allOf:
> > - $ref: usb-drd.yaml#
> > - if:
> > properties:
> > dr_mode:
> > const: host
> > then:
> > $ref: usb-hcd.yaml# (or usb-xhci.yaml#)
> > else:
> > #ref: usb.yaml#
> >
> > What do you think? @Rob, @Greg, we need your opinion here.
> >
> > -Sergey
> >
> > >
> > > > 2 files changed, 88 insertions(+), 57 deletions(-)
> > > > delete mode 100644 Documentation/devicetree/bindings/usb/generic.txt
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/usb/generic.txt b/Documentation/devicetree/bindings/usb/generic.txt
> > > > deleted file mode 100644
> > > > index ba472e7aefc9..000000000000
> > > > --- a/Documentation/devicetree/bindings/usb/generic.txt
> > > > +++ /dev/null
> > > > @@ -1,57 +0,0 @@
> > > > -Generic USB Properties
> > > > -
> > > > -Optional properties:
> > > > - - maximum-speed: tells USB controllers we want to work up to a certain
> > > > - speed. Valid arguments are "super-speed-plus",
> > > > - "super-speed", "high-speed", "full-speed" and
> > > > - "low-speed". In case this isn't passed via DT, USB
> > > > - controllers should default to their maximum HW
> > > > - capability.
> > > > - - dr_mode: tells Dual-Role USB controllers that we want to work on a
> > > > - particular mode. Valid arguments are "host",
> > > > - "peripheral" and "otg". In case this attribute isn't
> > > > - passed via DT, USB DRD controllers should default to
> > > > - OTG.
> > > > - - phy_type: tells USB controllers that we want to configure the core to support
> > > > - a UTMI+ PHY with an 8- or 16-bit interface if UTMI+ is
> > > > - selected. Valid arguments are "utmi" and "utmi_wide".
> > > > - In case this isn't passed via DT, USB controllers should
> > > > - default to HW capability.
> > > > - - otg-rev: tells usb driver the release number of the OTG and EH supplement
> > > > - with which the device and its descriptors are compliant,
> > > > - in binary-coded decimal (i.e. 2.0 is 0200H). This
> > > > - property is used if any real OTG features(HNP/SRP/ADP)
> > > > - is enabled, if ADP is required, otg-rev should be
> > > > - 0x0200 or above.
> > > > - - companion: phandle of a companion
> > > > - - hnp-disable: tells OTG controllers we want to disable OTG HNP, normally HNP
> > > > - is the basic function of real OTG except you want it
> > > > - to be a srp-capable only B device.
> > > > - - srp-disable: tells OTG controllers we want to disable OTG SRP, SRP is
> > > > - optional for OTG device.
> > > > - - adp-disable: tells OTG controllers we want to disable OTG ADP, ADP is
> > > > - optional for OTG device.
> > > > - - usb-role-switch: boolean, indicates that the device is capable of assigning
> > > > - the USB data role (USB host or USB device) for a given
> > > > - USB connector, such as Type-C, Type-B(micro).
> > > > - see connector/usb-connector.yaml.
> > > > - - role-switch-default-mode: indicating if usb-role-switch is enabled, the
> > > > - device default operation mode of controller while usb
> > > > - role is USB_ROLE_NONE. Valid arguments are "host" and
> > > > - "peripheral". Defaults to "peripheral" if not
> > > > - specified.
> > > > -
> > > > -
> > > > -This is an attribute to a USB controller such as:
> > > > -
> > > > -dwc3@4a030000 {
> > > > - compatible = "synopsys,dwc3";
> > > > - reg = <0x4a030000 0xcfff>;
> > > > - interrupts = <0 92 4>
> > > > - usb-phy = <&usb2_phy>, <&usb3,phy>;
> > > > - maximum-speed = "super-speed";
> > > > - dr_mode = "otg";
> > > > - phy_type = "utmi_wide";
> > > > - otg-rev = <0x0200>;
> > > > - adp-disable;
> > > > -};
> > > > diff --git a/Documentation/devicetree/bindings/usb/usb-hcd.yaml b/Documentation/devicetree/bindings/usb/usb-hcd.yaml
> > > > index 7263b7f2b510..ee7ea205c71d 100644
> > > > --- a/Documentation/devicetree/bindings/usb/usb-hcd.yaml
> > > > +++ b/Documentation/devicetree/bindings/usb/usb-hcd.yaml
> > > > @@ -22,9 +22,97 @@ properties:
> > > > description:
> > > > Name specifier for the USB PHY
> > > >
> > > > + maximum-speed:
> > > > + description:
> > > > + Tells USB controllers we want to work up to a certain speed. In case this
> > > > + isn't passed via DT, USB controllers should default to their maximum HW
> > > > + capability.
> > > > + $ref: /schemas/types.yaml#/definitions/string
> > > > + enum:
> > > > + - low-speed
> > > > + - full-speed
> > > > + - high-speed
> > > > + - super-speed
> > > > + - super-speed-plus
> > > > +
> > > > + dr_mode:
> > > > + description:
> > > > + Tells Dual-Role USB controllers that we want to work on a particular
> > > > + mode. In case this attribute isn't passed via DT, USB DRD controllers
> > > > + should default to OTG.
> > > > + $ref: /schemas/types.yaml#/definitions/string
> > > > + enum: [host, peripheral, otg]
> > > > +
> > > > + phy_type:
> > > > + description:
> > > > + Tells USB controllers that we want to configure the core to support a
> > > > + UTMI+ PHY with an 8- or 16-bit interface if UTMI+ is selected. In case
> > > > + this isn't passed via DT, USB controllers should default to HW
> > > > + capability.
> > > > + $ref: /schemas/types.yaml#/definitions/string
> > > > + enum: [utmi, utmi_wide]
> > > > +
> > > > + otg-rev:
> > > > + description:
> > > > + Tells usb driver the release number of the OTG and EH supplement with
> > > > + which the device and its descriptors are compliant, in binary-coded
> > > > + decimal (i.e. 2.0 is 0200H). This property is used if any real OTG
> > > > + features (HNP/SRP/ADP) is enabled. If ADP is required, otg-rev should be
> > > > + 0x0200 or above.
> > > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > > +
> > > > + companion:
> > > > + description: Phandle of a companion device
> > > > + $ref: /schemas/types.yaml#/definitions/phandle
> > > > +
> > > > + hnp-disable:
> > > > + description:
> > > > + Tells OTG controllers we want to disable OTG HNP. Normally HNP is the
> > > > + basic function of real OTG except you want it to be a srp-capable only B
> > > > + device.
> > > > + type: boolean
> > > > +
> > > > + srp-disable:
> > > > + description:
> > > > + Tells OTG controllers we want to disable OTG SRP. SRP is optional for OTG
> > > > + device.
> > > > + type: boolean
> > > > +
> > > > + adp-disable:
> > > > + description:
> > > > + Tells OTG controllers we want to disable OTG ADP. ADP is optional for OTG
> > > > + device.
> > > > + type: boolean
> > > > +
> > > > + usb-role-switch:
> > > > + description:
> > > > + Indicates that the device is capable of assigning the USB data role
> > > > + (USB host or USB device) for a given USB connector, such as Type-C,
> > > > + Type-B(micro). See connector/usb-connector.yaml.
> > > > +
> > > > + role-switch-default-mode:
> > > > + description:
> > > > + Indicates if usb-role-switch is enabled, the device default operation
> > > > + mode of controller while usb role is USB_ROLE_NONE.
> > > > + $ref: /schemas/types.yaml#/definitions/string
> > > > + enum: [host, peripheral]
> > > > + default: peripheral
> > > > +
> > > > examples:
> > > > - |
> > > > usb {
> > > > phys = <&usb2_phy1>, <&usb3_phy1>;
> > > > phy-names = "usb";
> > > > };
> > > > + - |
> > > > + usb@4a030000 {
> > > > + compatible = "snps,dwc3";
> > > > + reg = <0x4a030000 0xcfff>;
> > > > + interrupts = <0 92 4>;
> > > > + usb-phy = <&usb2_phy>, <&usb3_phy>;
> > > > + maximum-speed = "super-speed";
> > > > + dr_mode = "otg";
> > > > + phy_type = "utmi_wide";
> > > > + otg-rev = <0x0200>;
> > > > + adp-disable;
> > > > + };
> > >
^ permalink raw reply
* Re: [PATCH v2 2/2] dt-bindings: misc: convert fsl, qoriq-mc from txt to YAML
From: Rob Herring @ 2020-11-10 17:20 UTC (permalink / raw)
To: Laurentiu Tudor
Cc: devicetree, Jonathan Corbet, netdev, Linux Doc Mailing List,
linux-kernel@vger.kernel.org, Yang-Leo Li, Ioana Ciornei,
Ionut-robert Aron, Jakub Kicinski, linuxppc-dev, David Miller,
linux-arm-kernel
In-Reply-To: <20201109221123.GA1846668@bogus>
On Mon, Nov 9, 2020 at 4:11 PM Rob Herring <robh@kernel.org> wrote:
>
> On Mon, 09 Nov 2020 12:46:35 +0200, Laurentiu Tudor wrote:
> > From: Ionut-robert Aron <ionut-robert.aron@nxp.com>
> >
> > Convert fsl,qoriq-mc to YAML in order to automate the verification
> > process of dts files. In addition, update MAINTAINERS accordingly
> > and, while at it, add some missing files.
> >
> > Signed-off-by: Ionut-robert Aron <ionut-robert.aron@nxp.com>
> > [laurentiu.tudor@nxp.com: update MINTAINERS, updates & fixes in schema]
> > Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
> > ---
> > Changes in v2:
> > - fixed errors reported by yamllint
> > - dropped multiple unnecessary quotes
> > - used schema instead of text in description
> > - added constraints on dpmac reg property
> >
> > .../devicetree/bindings/misc/fsl,qoriq-mc.txt | 196 ----------------
> > .../bindings/misc/fsl,qoriq-mc.yaml | 210 ++++++++++++++++++
> > .../ethernet/freescale/dpaa2/overview.rst | 5 +-
> > MAINTAINERS | 4 +-
> > 4 files changed, 217 insertions(+), 198 deletions(-)
> > delete mode 100644 Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
> > create mode 100644 Documentation/devicetree/bindings/misc/fsl,qoriq-mc.yaml
> >
>
> Applied, thanks!
And now dropped. This duplicates what's in commit 0dbcd4991719
("dt-bindings: net: add the DPAA2 MAC DTS definition") and has
warnings from it:
/builds/robherring/linux-dt-bindings/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.example.dt.yaml:
dpmac@1: $nodename:0: 'dpmac@1' does not match '^ethernet(@.*)?$'
From schema: /builds/robherring/linux-dt-bindings/Documentation/devicetree/bindings/net/fsl,qoriq-mc-dpmac.yaml
Rob
^ permalink raw reply
* Re: [PATCH v2 2/2] dt-bindings: misc: convert fsl, qoriq-mc from txt to YAML
From: Laurentiu Tudor @ 2020-11-10 17:35 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree, Jonathan Corbet, netdev, Linux Doc Mailing List,
linux-kernel@vger.kernel.org, Yang-Leo Li, Ioana Ciornei,
Ionut-robert Aron, Jakub Kicinski, linuxppc-dev, David Miller,
linux-arm-kernel
In-Reply-To: <CAL_JsqJ2Ew6GdQmE0gcTgFX9cMZKtkL_rO1F+0EMNy88wF+gXw@mail.gmail.com>
On 11/10/2020 7:20 PM, Rob Herring wrote:
> On Mon, Nov 9, 2020 at 4:11 PM Rob Herring <robh@kernel.org> wrote:
>>
>> On Mon, 09 Nov 2020 12:46:35 +0200, Laurentiu Tudor wrote:
>>> From: Ionut-robert Aron <ionut-robert.aron@nxp.com>
>>>
>>> Convert fsl,qoriq-mc to YAML in order to automate the verification
>>> process of dts files. In addition, update MAINTAINERS accordingly
>>> and, while at it, add some missing files.
>>>
>>> Signed-off-by: Ionut-robert Aron <ionut-robert.aron@nxp.com>
>>> [laurentiu.tudor@nxp.com: update MINTAINERS, updates & fixes in schema]
>>> Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
>>> ---
>>> Changes in v2:
>>> - fixed errors reported by yamllint
>>> - dropped multiple unnecessary quotes
>>> - used schema instead of text in description
>>> - added constraints on dpmac reg property
>>>
>>> .../devicetree/bindings/misc/fsl,qoriq-mc.txt | 196 ----------------
>>> .../bindings/misc/fsl,qoriq-mc.yaml | 210 ++++++++++++++++++
>>> .../ethernet/freescale/dpaa2/overview.rst | 5 +-
>>> MAINTAINERS | 4 +-
>>> 4 files changed, 217 insertions(+), 198 deletions(-)
>>> delete mode 100644 Documentation/devicetree/bindings/misc/fsl,qoriq-mc.txt
>>> create mode 100644 Documentation/devicetree/bindings/misc/fsl,qoriq-mc.yaml
>>>
>>
>> Applied, thanks!
>
> And now dropped. This duplicates what's in commit 0dbcd4991719
> ("dt-bindings: net: add the DPAA2 MAC DTS definition") and has
> warnings from it:
>
> /builds/robherring/linux-dt-bindings/Documentation/devicetree/bindings/misc/fsl,qoriq-mc.example.dt.yaml:
> dpmac@1: $nodename:0: 'dpmac@1' does not match '^ethernet(@.*)?$'
> From schema: /builds/robherring/linux-dt-bindings/Documentation/devicetree/bindings/net/fsl,qoriq-mc-dpmac.yaml
>
My patch converts the .txt devicetree/bindings/misc/fsl,qoriq-mc.yaml
while the commit you mention creates
devicetree/bindings/net/fsl,qoriq-mc-dpmac.yaml, but at a first sight
there seems to be some duplication. Will sync internally with my
colleagues and return with a resolution. Thanks and sorry for the trouble.
---
Best Regards, Laurentiu
^ permalink raw reply
* Re: [PATCH] powerpc/pseries/hotplug-cpu: Fix memleak when cpus node not exist
From: Tyrel Datwyler @ 2020-11-10 17:47 UTC (permalink / raw)
To: Nathan Lynch, Zhang Xiaoxu; +Cc: linuxppc-dev, paulus, groug
In-Reply-To: <87ft5hjocy.fsf@linux.ibm.com>
On 11/10/20 6:08 AM, Nathan Lynch wrote:
> Zhang Xiaoxu <zhangxiaoxu5@huawei.com> writes:
>> From: zhangxiaoxu <zhangxiaoxu5@huawei.com>
>>
>> If the cpus nodes not exist, we lost to free the 'cpu_drcs', which
>> will leak memory.
>>
>> Fixes: a0ff72f9f5a7 ("powerpc/pseries/hotplug-cpu: Remove double free in error path")
>> Reported-by: Hulk Robot <hulkci@huawei.com>
>> Signed-off-by: zhangxiaoxu <zhangxiaoxu5@huawei.com>
>> ---
>> arch/powerpc/platforms/pseries/hotplug-cpu.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
>> index f2837e33bf5d..4bb1c9f2bb11 100644
>> --- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
>> +++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
>> @@ -743,6 +743,7 @@ static int dlpar_cpu_add_by_count(u32 cpus_to_add)
>> parent = of_find_node_by_path("/cpus");
>> if (!parent) {
>> pr_warn("Could not find CPU root node in device tree\n");
>> + kfree(cpu_drcs);
>> return -1;
>> }
>
> Thanks for finding this.
>
> a0ff72f9f5a7 ("powerpc/pseries/hotplug-cpu: Remove double free in error
> path") was posted in Sept 2019 but was not applied until July 2020:
>
> https://lore.kernel.org/linuxppc-dev/20190919231633.1344-1-nathanl@linux.ibm.com/
>
> Here is that change as posted; note the function context is
> find_dlpar_cpus_to_add(), not dlpar_cpu_add_by_count():
>
> --- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
> +++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
> @@ -726,7 +726,6 @@ static int find_dlpar_cpus_to_add(u32 *cpu_drcs, u32 cpus_to_add)
> parent = of_find_node_by_path("/cpus");
> if (!parent) {
> pr_warn("Could not find CPU root node in device tree\n");
> - kfree(cpu_drcs);
> return -1;
> }
>
> Meanwhile b015f6bc9547dbc056edde7177c7868ca8629c4c ("powerpc/pseries: Add
> cpu DLPAR support for drc-info property") was posted in Nov 2019 and
> committed a few days later:
>
> https://lore.kernel.org/linux-pci/1573449697-5448-4-git-send-email-tyreld@linux.ibm.com/
>
> This change reorganized the same code, removing
> find_dlpar_cpus_to_add(), and it had the effect of fixing the same
> issue.
>
> However git apparently allowed the older change to still apply on top of
> this (changing a function different from the one in the original
> patch!), leading to a real bug.
Yikes, not sure how that happened without either the committer massaging the
patch to apply, or the line location and context matching exactly.
>
> Your patch is correct but it should be framed as a revert of
> a0ff72f9f5a7 with this context in the commit message.
>
Agreed, in reality we want to revert a patch that shouldn't have been applied.
-Tyrel
^ permalink raw reply
* [PATCH] powerpc/pseries/hotplug-cpu: Fix memleak when cpus node not exist
From: Zhang Xiaoxu @ 2020-11-10 12:30 UTC (permalink / raw)
To: zhangxiaoxu5, nathanl, mpe, benh, paulus, groug, linuxppc-dev
From: zhangxiaoxu <zhangxiaoxu5@huawei.com>
If the cpus nodes not exist, we lost to free the 'cpu_drcs', which
will leak memory.
Fixes: a0ff72f9f5a7 ("powerpc/pseries/hotplug-cpu: Remove double free in error path")
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: zhangxiaoxu <zhangxiaoxu5@huawei.com>
---
arch/powerpc/platforms/pseries/hotplug-cpu.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
index f2837e33bf5d..4bb1c9f2bb11 100644
--- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
+++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
@@ -743,6 +743,7 @@ static int dlpar_cpu_add_by_count(u32 cpus_to_add)
parent = of_find_node_by_path("/cpus");
if (!parent) {
pr_warn("Could not find CPU root node in device tree\n");
+ kfree(cpu_drcs);
return -1;
}
--
2.25.4
^ permalink raw reply related
* Re: Duplicated ABI entries - Was: Re: [PATCH v2 20/39] docs: ABI: testing: make the files compatible with ReST output
From: Randy Dunlap @ 2020-11-10 18:18 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Jonathan Cameron
Cc: Gautham R. Shenoy, Jason A. Donenfeld, Heikki Krogerus,
Peter Meerwald-Stadler, Petr Mladek, Linux Doc Mailing List,
Alexander Shishkin, Nayna Jain, Jonathan Cameron,
Alexandre Belloni, Mimi Zohar, Sebastian Reichel, linux-mm,
Bruno Meneguele, Vishal Verma, Pavel Machek, Hanjun Guo,
Guenter Roeck, netdev, Oleh Kravchenko, Dan Williams,
Andrew Donnellan, Javier González, Fabrice Gasnier,
Mark Gross, linux-acpi, Jonathan Corbet, Chunyan Zhang,
Mario Limonciello, linux-stm32, Lakshmi Ramasubramanian,
Ludovic Desroches, Pawan Gupta, linux-arm-kernel, Tom Rix,
Frederic Barrat, Niklas Cassel, Len Brown, Juergen Gross,
linuxppc-dev, Mika Westerberg, Alexandre Torgue, linux-pm,
linux-kernel, Richard Cochran, Oded Gabbay, Baolin Wang,
Lars-Peter Clausen, Dan Murphy, Orson Zhai, Philippe Bergheaud,
xen-devel, Boris Ostrovsky, Andy Shevchenko, Benson Leung,
Konstantin Khlebnikov, Jens Axboe, Felipe Balbi, Kranthi Kuntala,
Martin K. Petersen, Greg Kroah-Hartman, linux-usb,
Rafael J. Wysocki, Nicolas Ferre, linux-iio, Thinh Nguyen,
Sergey Senozhatsky, Stefano Stabellini, Thomas Gleixner,
Leonid Maksymchuk, Maxime Coquelin, Johannes Thumshirn,
Enric Balletbo i Serra, Vaibhav Jain, Vineela Tummalapalli,
Peter Rosin, Mike Kravetz
In-Reply-To: <20201110082658.2edc1ab5@coco.lan>
On 11/9/20 11:26 PM, Mauro Carvalho Chehab wrote:
> Hi Jonathan,
>
> Let's view ABI from the PoV of a system admin that doesn't know
> yet about a certain ABI symbol.
>
> He'll try to seek for the symbol, more likely using the HTML
> documentation. Only very senior system admins might try to take
> a look at the Kernel.
FWIW, I think that the likely search methods are $search_engine
and 'grep'.
Have a good few days off.
--
~Randy
^ permalink raw reply
* Re: [PATCH] ASoC: fsl_xcvr: fix break condition
From: Mark Brown @ 2020-11-10 21:38 UTC (permalink / raw)
To: Xiubo Li, Timur Tabi, Nicolin Chen, Fabio Estevam, Shengjiu Wang,
alsa-devel, linux-kernel, linuxppc-dev, Jaroslav Kysela,
Liam Girdwood, Viorel Suman (OSS), Takashi Iwai
Cc: Viorel Suman
In-Reply-To: <20201102161810.902464-1-viorel.suman@oss.nxp.com>
On Mon, 2 Nov 2020 18:18:10 +0200, Viorel Suman (OSS) wrote:
> The break condition copied by mistake as same
> as loop condition in the previous version, but must
> be the opposite. So fix it.
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/1] ASoC: fsl_xcvr: fix break condition
commit: 048751de568816de52dedf0fa967cceada7885f1
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
^ permalink raw reply
* Re: [PATCH] KVM: PPC: Book3S HV: XIVE: Fix possible oops when accessing ESB page
From: Cédric Le Goater @ 2020-11-10 22:04 UTC (permalink / raw)
To: Michael Ellerman, Paul Mackerras
Cc: kvm, Gustavo Romero, Greg Kurz, kvm-ppc, linuxppc-dev,
David Gibson
In-Reply-To: <878sbftbnt.fsf@mpe.ellerman.id.au>
On 11/6/20 4:19 AM, Michael Ellerman wrote:
> Cédric Le Goater <clg@kaod.org> writes:
>> When accessing the ESB page of a source interrupt, the fault handler
>> will retrieve the page address from the XIVE interrupt 'xive_irq_data'
>> structure. If the associated KVM XIVE interrupt is not valid, that is
>> not allocated at the HW level for some reason, the fault handler will
>> dereference a NULL pointer leading to the oops below :
>>
>> WARNING: CPU: 40 PID: 59101 at arch/powerpc/kvm/book3s_xive_native.c:259 xive_native_esb_fault+0xe4/0x240 [kvm]
>> CPU: 40 PID: 59101 Comm: qemu-system-ppc Kdump: loaded Tainted: G W --------- - - 4.18.0-240.el8.ppc64le #1
>> NIP: c00800000e949fac LR: c00000000044b164 CTR: c00800000e949ec8
>> REGS: c000001f69617840 TRAP: 0700 Tainted: G W --------- - - (4.18.0-240.el8.ppc64le)
>> MSR: 9000000000029033 <SF,HV,EE,ME,IR,DR,RI,LE> CR: 44044282 XER: 00000000
>> CFAR: c00000000044b160 IRQMASK: 0
>> GPR00: c00000000044b164 c000001f69617ac0 c00800000e96e000 c000001f69617c10
>> GPR04: 05faa2b21e000080 0000000000000000 0000000000000005 ffffffffffffffff
>> GPR08: 0000000000000000 0000000000000001 0000000000000000 0000000000000001
>> GPR12: c00800000e949ec8 c000001ffffd3400 0000000000000000 0000000000000000
>> GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>> GPR20: 0000000000000000 0000000000000000 c000001f5c065160 c000000001c76f90
>> GPR24: c000001f06f20000 c000001f5c065100 0000000000000008 c000001f0eb98c78
>> GPR28: c000001dcab40000 c000001dcab403d8 c000001f69617c10 0000000000000011
>> NIP [c00800000e949fac] xive_native_esb_fault+0xe4/0x240 [kvm]
>> LR [c00000000044b164] __do_fault+0x64/0x220
>> Call Trace:
>> [c000001f69617ac0] [0000000137a5dc20] 0x137a5dc20 (unreliable)
>> [c000001f69617b50] [c00000000044b164] __do_fault+0x64/0x220
>> [c000001f69617b90] [c000000000453838] do_fault+0x218/0x930
>> [c000001f69617bf0] [c000000000456f50] __handle_mm_fault+0x350/0xdf0
>> [c000001f69617cd0] [c000000000457b1c] handle_mm_fault+0x12c/0x310
>> [c000001f69617d10] [c00000000007ef44] __do_page_fault+0x264/0xbb0
>> [c000001f69617df0] [c00000000007f8c8] do_page_fault+0x38/0xd0
>> [c000001f69617e30] [c00000000000a714] handle_page_fault+0x18/0x38
>> Instruction dump:
>> 40c2fff0 7c2004ac 2fa90000 409e0118 73e90001 41820080 e8bd0008 7c2004ac
>> 7ca90074 39400000 915c0000 7929d182 <0b090000> 2fa50000 419e0080 e89e0018
>> ---[ end trace 66c6ff034c53f64f ]---
>> xive-kvm: xive_native_esb_fault: accessing invalid ESB page for source 8 !
>>
>> Fix that by checking the validity of the KVM XIVE interrupt structure.
>>
>> Reported-by: Greg Kurz <groug@kaod.org>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>
> Fixes ?
Ah yes :/
Cc: stable@vger.kernel.org # v5.2+
Fixes: 6520ca64cde7 ("KVM: PPC: Book3S HV: XIVE: Add a mapping for the source ESB pages")
Since my provider changed its imap servers, my email filters are really screwed
up and I miss emails.
Sorry about that,
C.
^ permalink raw reply
* [PATCH] Revert "powerpc/pseries/hotplug-cpu: Remove double free in error path"
From: Zhang Xiaoxu @ 2020-11-11 2:07 UTC (permalink / raw)
To: tyreld, zhangxiaoxu5, linuxppc-dev, mpe, benh, paulus, groug
This reverts commit a0ff72f9f5a780341e7ff5e9ba50a0dad5fa1980.
Since the commit b015f6bc9547 ("powerpc/pseries: Add cpu DLPAR
support for drc-info property"), the 'cpu_drcs' wouldn't be double
freed when the 'cpus' node not found.
So we needn't apply this patch, otherwise, the memory will be leak.
Fixes: a0ff72f9f5a7 ("powerpc/pseries/hotplug-cpu: Remove double free in error path")
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Zhang Xiaoxu <zhangxiaoxu5@huawei.com>
---
arch/powerpc/platforms/pseries/hotplug-cpu.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c b/arch/powerpc/platforms/pseries/hotplug-cpu.c
index f2837e33bf5d..4bb1c9f2bb11 100644
--- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
+++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
@@ -743,6 +743,7 @@ static int dlpar_cpu_add_by_count(u32 cpus_to_add)
parent = of_find_node_by_path("/cpus");
if (!parent) {
pr_warn("Could not find CPU root node in device tree\n");
+ kfree(cpu_drcs);
return -1;
}
--
2.25.4
^ permalink raw reply related
* Re: [PATCH 02/18] powerpc: remove arguments from fault handler functions
From: Nicholas Piggin @ 2020-11-11 4:45 UTC (permalink / raw)
To: Christophe Leroy, linuxppc-dev
In-Reply-To: <3872d710-97e2-80c3-991c-7f1ffe790a3d@csgroup.eu>
Excerpts from Christophe Leroy's message of November 10, 2020 9:15 pm:
>
>
> Le 10/11/2020 à 09:29, Nicholas Piggin a écrit :
>> Excerpts from Christophe Leroy's message of November 6, 2020 5:59 pm:
>>>
>>>
>>> Le 05/11/2020 à 15:34, Nicholas Piggin a écrit :
>>>> Make mm fault handlers all just take the pt_regs * argument and load
>>>> DAR/DSISR from that. Make those that return a value return long.
>>>>
>>>> This is done to make the function signatures match other handlers, which
>>>> will help with a future patch to add wrappers. Explicit arguments could
>>>> be added for performance but that would require more wrapper macro
>>>> variants.
>>>>
>>>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>>>> ---
>
> [...]
>
>>
>>>> diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
>>>> index e65a49f246ef..390a296b16a3 100644
>>>> --- a/arch/powerpc/mm/fault.c
>>>> +++ b/arch/powerpc/mm/fault.c
>>>> @@ -549,11 +549,12 @@ static int __do_page_fault(struct pt_regs *regs, unsigned long address,
>>>> }
>>>> NOKPROBE_SYMBOL(__do_page_fault);
>>>>
>>>> -int do_page_fault(struct pt_regs *regs, unsigned long address,
>>>> - unsigned long error_code)
>>>> +long do_page_fault(struct pt_regs *regs)
>>>> {
>>>> enum ctx_state prev_state = exception_enter();
>>>> - int err;
>>>> + unsigned long address = regs->dar;
>>>> + unsigned long error_code = regs->dsisr;
>>>> + long err;
>>>
>>> By doing something more or less like this (need to be tuned for bookE as well):
>>>
>>> + int is_exec = TRAP(regs) == 0x400;
>>> + unsigned long address = is_exec ? regs->ssr0 : regs->dar;
>>> + unsigned long error_code = is_exec ? (regs->ssr1 & DSISR_SRR1_MATCH_32S) : regs->dsisr;
>>
>> Ah, I didn't see that you saved these in srr0/1 already. Hmm, not in
>> pt_regs though. thread_struct (VMAP_STACK only)? exception_regs (booke
>> only)? Doesn't seem so easy.
>
> Oops yes you are right, SRR0/SRR1 are not in pt_regs. And their validity in thread struct is rather
> short ... So forget my comment.
So, are you happy to go with this for now? I guess things can
later be cleaned up to avoid double saving on cases like VMAP.
Thanks,
Nick
^ 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