LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH kernel v3] powerpc/makefile: Do not redefine $(CPP) for preprocessor
From: Segher Boessenkool @ 2021-05-17 19:10 UTC (permalink / raw)
  To: Alexey Kardashevskiy
  Cc: Michal Marek, Linux Kbuild mailing list, Masahiro Yamada,
	Nick Desaulniers, Linux Kernel Mailing List, Nathan Chancellor,
	clang-built-linux, linuxppc-dev
In-Reply-To: <f8d64d7d-cd2b-342a-de6a-c14a5df9462c@ozlabs.ru>

Hi!

On Mon, May 17, 2021 at 01:23:11PM +1000, Alexey Kardashevskiy wrote:
> On 5/14/21 18:46, Segher Boessenkool wrote:
> >On Fri, May 14, 2021 at 11:42:32AM +0900, Masahiro Yamada wrote:
> >>In my best guess, the reason why powerpc adding the endian flag to CPP
> >>is this line in arch/powerpc/kernel/vdso64/vdso64.lds.S
> >>
> >>#ifdef __LITTLE_ENDIAN__
> >>OUTPUT_FORMAT("elf64-powerpcle", "elf64-powerpcle", "elf64-powerpcle")
> >>#else
> >>OUTPUT_FORMAT("elf64-powerpc", "elf64-powerpc", "elf64-powerpc")
> >>#endif
> >
> >Which is equivalent to
> >
> >#ifdef __LITTLE_ENDIAN__
> >OUTPUT_FORMAT("elf64-powerpcle")
> >#else
> >OUTPUT_FORMAT("elf64-powerpc")
> >#endif
> >
> >so please change that at the same time if you touch this :-)
> 
> "If you touch this" approach did not work well with this patch so sorry 
> but no ;)
> 
> and for a separate patch, I'll have to dig since when it is equal, do 
> you know?

Since 1994, when the three-arg version was introduced (the one-arg
version is from 1992).

> >>__LITTLE_ENDIAN__  is defined by powerpc gcc and clang.
> >
> >This predefined macro is required by the newer ABIs, but all older
> 
> That's good so I'll stick to it.

Great.

> >You can just write -mbig and -mlittle btw.  Those aren't available on
> >all targets, but neither are the long-winded -m{big,little}-endian
> >option names.  Pet peeve, I know :-)
> 
> I am looking the same guarantees across modern enough gcc and clang and 
> I am not sure all of the above is valid for clang 10.0.something (or 
> whatever we say we support) ;)

-mbig/-mlittle is supported in GCC since times immemorial.  Whether LLVM
supports it as well just depends on how good their emulation is, I have
no idea.


Segher

^ permalink raw reply

* Re: [PATCH v2 01/14] PCI: Use sysfs_emit() and sysfs_emit_at() in "show" functions
From: Krzysztof Wilczyński @ 2021-05-17 17:44 UTC (permalink / raw)
  To: Logan Gunthorpe
  Cc: Tyrel Datwyler, linux-pci, Vidya Sagar, Joe Perches,
	Oliver O'Halloran, Bjorn Helgaas, Paul Mackerras,
	Kurt Schwemmer, linuxppc-dev, Xiongfeng Wang
In-Reply-To: <b3be37d4-5d98-474e-05ca-afce4782c359@deltatee.com>

Hi Logan,

> > The sysfs_emit() and sysfs_emit_at() functions were introduced to make
> > it less ambiguous which function is preferred when writing to the output
> > buffer in a device attribute's "show" callback [1].
> > 
> > Convert the PCI sysfs object "show" functions from sprintf(), snprintf()
> > and scnprintf() to sysfs_emit() and sysfs_emit_at() accordingly, as the
> > latter is aware of the PAGE_SIZE buffer and correctly returns the number
> > of bytes written into the buffer.
> > 
> > No functional change intended.
> > 
> > [1] Documentation/filesystems/sysfs.rst
> > 
> > Related to:
> >   commit ad025f8e46f3 ("PCI/sysfs: Use sysfs_emit() and sysfs_emit_at() in "show" functions")
> 
> I re-reviewed the whole series. It still looks good to me.
> 
> Very nice solution in patch 12 to the new line issue.
> 
> Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
> 
> Thanks,

Thank you!

I will send v3 incorporating the style change as per Joe's suggestion
and carry-over your "Reviewed-by", if you don't mind, as it will be
a trivial change.

Krzysztof

^ permalink raw reply

* Re: [PATCH v2 01/14] PCI: Use sysfs_emit() and sysfs_emit_at() in "show" functions
From: Logan Gunthorpe @ 2021-05-17 15:48 UTC (permalink / raw)
  To: Krzysztof Wilczyński, Bjorn Helgaas
  Cc: Tyrel Datwyler, linux-pci, Vidya Sagar, Oliver O'Halloran,
	Joe Perches, Paul Mackerras, Kurt Schwemmer, linuxppc-dev,
	Xiongfeng Wang
In-Reply-To: <20210515052434.1413236-1-kw@linux.com>



On 2021-05-14 11:24 p.m., Krzysztof Wilczyński wrote:
> The sysfs_emit() and sysfs_emit_at() functions were introduced to make
> it less ambiguous which function is preferred when writing to the output
> buffer in a device attribute's "show" callback [1].
> 
> Convert the PCI sysfs object "show" functions from sprintf(), snprintf()
> and scnprintf() to sysfs_emit() and sysfs_emit_at() accordingly, as the
> latter is aware of the PAGE_SIZE buffer and correctly returns the number
> of bytes written into the buffer.
> 
> No functional change intended.
> 
> [1] Documentation/filesystems/sysfs.rst
> 
> Related to:
>   commit ad025f8e46f3 ("PCI/sysfs: Use sysfs_emit() and sysfs_emit_at() in "show" functions")
> 
> Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
> Reviewed-by: Logan Gunthorpe <logang@deltatee.com>

I re-reviewed the whole series. It still looks good to me.

Very nice solution in patch 12 to the new line issue.

Reviewed-by: Logan Gunthorpe <logang@deltatee.com>

Thanks,

Logan

^ permalink raw reply

* Re: Fwd: [Bug 213069] New: kernel BUG at arch/powerpc/include/asm/book3s/64/hash-4k.h:147! Oops: Exception in kernel mode, sig: 5 [#1]
From: Christophe Leroy @ 2021-05-17 15:26 UTC (permalink / raw)
  To: Anshuman Khandual, Aneesh Kumar K.V,
	linuxppc-dev@lists.ozlabs.org
In-Reply-To: <69fb90be-8b30-aadc-0fcf-b1ce9fc1a68f@arm.com>



Le 17/05/2021 à 15:12, Anshuman Khandual a écrit :
> 
> 
> On 5/17/21 6:29 PM, Christophe Leroy wrote:
>>
>>
>> Le 17/05/2021 à 14:49, Anshuman Khandual a écrit :
>>>
>>>
>>> On 5/17/21 11:25 AM, Aneesh Kumar K.V wrote:
>>>> On 5/17/21 11:17 AM, Christophe Leroy wrote:
>>>>> +aneesh
>>>>> +linuxppc-dev list
>>>>>
>>>>> Le 17/05/2021 à 07:44, Anshuman Khandual a écrit :
>>>>>> Hello Christophe,
>>>>>>
>>>>>> DEBUG_VM_PGTABLE has now been re-enabled on powerpc recently ? was not
>>>>>> aware about this. From the error log, it failed explicitly on 4K page
>>>>>> size hash config.
>>>>>>
>>>>>> static inline pmd_t hash__pmd_mkhuge(pmd_t pmd)
>>>>>> {
>>>>>>            BUG();        ------> Failed
>>>>>>            return pmd;
>>>>>> }
>>>>>>
>>>>>> static inline pmd_t __pmd_mkhuge(pmd_t pmd)
>>>>>> {
>>>>>>            if (radix_enabled())
>>>>>>                    return radix__pmd_mkhuge(pmd);
>>>>>>            return hash__pmd_mkhuge(pmd);
>>>>>> }
>>>>>>
>>>>>> pmd_t pfn_pmd(unsigned long pfn, pgprot_t pgprot)
>>>>>> {
>>>>>>            unsigned long pmdv;
>>>>>>
>>>>>>            pmdv = (pfn << PAGE_SHIFT) & PTE_RPN_MASK;
>>>>>>
>>>>>>            return __pmd_mkhuge(pmd_set_protbits(__pmd(pmdv), pgprot));
>>>>>> }
>>>>>>
>>>>>> It seems like on powerpc, where pfn_pmd() makes a huge page but which
>>>>>> is not supported on 4K hash config thus triggering the BUG(). But all
>>>>>> pfn_pmd() call sites inside the debug_vm_pgtable() test are protected
>>>>>> with CONFIG_TRANSPARENT_HUGEPAGE. IIUC unlike powerpc, pfn_pmd() does
>>>>>> not directly make a huge page on other platforms.
>>>>>>
>>>>>> Looking at arch/powerpc/include/asm/book3s/64/hash-4k.h, all relevant
>>>>>> THP helpers has BUG() or 0 which indicates THP might not be supported
>>>>>> on 4K page size hash config ?
>>>>>>
>>>>>> But looking at arch/powerpc/platforms/Kconfig.cputype, it seems like
>>>>>> HAVE_ARCH_TRANSPARENT_HUGEPAGE is invariably selected on PPC_BOOK3S_64
>>>>>> platforms which I assume includes 4K page size hash config as well.
>>>>>>
>>>>>> Is THP some how getting enabled on this 4K page size hash config where
>>>>>> it should not be (thus triggering the BUG) ? OR am I missing something
>>>>>> here.
>>>>>>
>>>>>
>>>>
>>>> We should put those  pfn_pmd()  and pfn_pud() after
>>>>
>>>>       if (!has_transparent_hugepage())
>>>>           return;
>>>
>>> The following patch has been lightly tested on arm64 and x86 platforms.
>>> Could you please verify if this solves the problem on powerpc 4K hash
>>> config ? Thank you.
>>
>> No need to update pmd_advanced_tests() and pud_advanced_tests() ?
> 
> They already have has_transparent_hugepage() check and all pfn_pxx()
> instances are after that check. Do you see any other concern ?
> 

As far as I can see, in 5.13-rc2 they are before the check :

https://elixir.bootlin.com/linux/v5.13-rc2/source/mm/debug_vm_pgtable.c#L183

Christophe

^ permalink raw reply

* Re: [PATCH v5 5/9] powerpc/mm/book3s64: Update tlb flush routines to take a page walk cache flush argument
From: Guenter Roeck @ 2021-05-17 14:18 UTC (permalink / raw)
  To: Aneesh Kumar K.V; +Cc: npiggin, linux-mm, kaleshsingh, joel, akpm, linuxppc-dev
In-Reply-To: <87pmxpqxb1.fsf@linux.ibm.com>

On 5/17/21 6:55 AM, Aneesh Kumar K.V wrote:
> Guenter Roeck <linux@roeck-us.net> writes:
> 
>> On 5/17/21 1:40 AM, Aneesh Kumar K.V wrote:
>>> On 5/15/21 10:05 PM, Guenter Roeck wrote:
>>>> On Thu, Apr 22, 2021 at 11:13:19AM +0530, Aneesh Kumar K.V wrote:
> 
> ...
> 
>>>>    extern void radix__local_flush_all_mm(struct mm_struct *mm);
>>>>> diff --git a/arch/powerpc/include/asm/book3s/64/tlbflush.h b/arch/powerpc/include/asm/book3s/64/tlbflush.h
>>>>> index 215973b4cb26..f9f8a3a264f7 100644
>>>>> --- a/arch/powerpc/include/asm/book3s/64/tlbflush.h
>>>>> +++ b/arch/powerpc/include/asm/book3s/64/tlbflush.h
>>>>> @@ -45,13 +45,30 @@ static inline void tlbiel_all_lpid(bool radix)
>>>>>            hash__tlbiel_all(TLB_INVAL_SCOPE_LPID);
>>>>>    }
>>>>> +static inline void flush_pmd_tlb_pwc_range(struct vm_area_struct *vma,
>>>>                    ^^^^
>>>>> +                       unsigned long start,
>>>>> +                       unsigned long end,
>>>>> +                       bool flush_pwc)
>>>>> +{
>>>>> +    if (radix_enabled())
>>>>> +        return radix__flush_pmd_tlb_range(vma, start, end, flush_pwc);
>>>>> +    return hash__flush_tlb_range(vma, start, end);
>>>>           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>>
>>>>> +}
>>>
>>> In this specific case we won't have  build errors because,
>>>
>>> static inline void hash__flush_tlb_range(struct vm_area_struct *vma,
>>>                        unsigned long start, unsigned long end)
>>> {
>>>
>>
>> Sorry, you completely lost me.
>>
>> Building parisc:allnoconfig ... failed
>> --------------
>> Error log:
>> In file included from arch/parisc/include/asm/cacheflush.h:7,
>>                    from include/linux/highmem.h:12,
>>                    from include/linux/pagemap.h:11,
>>                    from include/linux/ksm.h:13,
>>                    from mm/mremap.c:14:
>> mm/mremap.c: In function 'flush_pte_tlb_pwc_range':
>> arch/parisc/include/asm/tlbflush.h:20:2: error: 'return' with a value, in function returning void
> 
> As replied here
> https://lore.kernel.org/mm-commits/8eedb441-a612-1ec8-8bf7-b40184de9f6f@linux.ibm.com/
> 
> That was the generic header change in the patch. I was commenting about the
> ppc64 specific change causing build failures.
> 

Ah, sorry. I wasn't aware that the following is valid C code

void f1()
{
     return f2();
     ^^^^^^
}

as long as f2() is void as well. Confusing, but we live and learn.

Guenter

^ permalink raw reply

* Re: [PATCH 6/9] tty: hvc_console: Fix coding style issues of block comments
From: Johan Hovold @ 2021-05-17 14:15 UTC (permalink / raw)
  To: Xiaofei Tan; +Cc: gregkh, linuxppc-dev, jirislaby, linux-kernel, linuxarm
In-Reply-To: <1621233433-27094-7-git-send-email-tanxiaofei@huawei.com>

On Mon, May 17, 2021 at 02:37:10PM +0800, Xiaofei Tan wrote:
> Fix coding style issues of block comments, reported by checkpatch.pl.
> Besides, add a period at the end of the sentenses.
> 
> Signed-off-by: Xiaofei Tan <tanxiaofei@huawei.com>
> ---
>  drivers/tty/hvc/hvc_console.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c
> index 39018e5..a61cdf0 100644
> --- a/drivers/tty/hvc/hvc_console.c
> +++ b/drivers/tty/hvc/hvc_console.c
> @@ -177,7 +177,8 @@ static void hvc_console_print(struct console *co, const char *b,
>  			r = cons_ops[index]->put_chars(vtermnos[index], c, i);
>  			if (r <= 0) {
>  				/* throw away characters on error
> -				 * but spin in case of -EAGAIN */
> +				 * but spin in case of -EAGAIN.
> +				 */

How is this an improvement? First, the multi-line comment style is

	/*
	 * ...
	 */

Second, that sentence is not capitalised so why do add a period?

Third, why are you sending checkpatch.pl cleanups for files outside of
staging?

Unless doing some real changes to the files in question as well this is
mostly just churn and noise that makes it harder to backport fixes and
do code forensics for no real gain.

Greg may disagree, but I don't think we should be encouraging this kind
of patches.

Johan

^ permalink raw reply

* Re: Fwd: [Bug 213069] New: kernel BUG at arch/powerpc/include/asm/book3s/64/hash-4k.h:147! Oops: Exception in kernel mode, sig: 5 [#1]
From: Anshuman Khandual @ 2021-05-17 14:12 UTC (permalink / raw)
  To: Aneesh Kumar K.V, Christophe Leroy, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <87tun1qxh8.fsf@linux.ibm.com>


On 5/17/21 7:22 PM, Aneesh Kumar K.V wrote:
> Anshuman Khandual <anshuman.khandual@arm.com> writes:
> 
>> On 5/17/21 11:25 AM, Aneesh Kumar K.V wrote:
>>> On 5/17/21 11:17 AM, Christophe Leroy wrote:
>>>> +aneesh
>>>> +linuxppc-dev list
>>>>
>>>> Le 17/05/2021 à 07:44, Anshuman Khandual a écrit :
>>>>> Hello Christophe,
>>>>>
>>>>> DEBUG_VM_PGTABLE has now been re-enabled on powerpc recently ? was not
>>>>> aware about this. From the error log, it failed explicitly on 4K page
>>>>> size hash config.
>>>>>
>>>>> static inline pmd_t hash__pmd_mkhuge(pmd_t pmd)
>>>>> {
>>>>>          BUG();        ------> Failed
>>>>>          return pmd;
>>>>> }
>>>>>
>>>>> static inline pmd_t __pmd_mkhuge(pmd_t pmd)
>>>>> {
>>>>>          if (radix_enabled())
>>>>>                  return radix__pmd_mkhuge(pmd);
>>>>>          return hash__pmd_mkhuge(pmd);
>>>>> }
>>>>>
>>>>> pmd_t pfn_pmd(unsigned long pfn, pgprot_t pgprot)
>>>>> {
>>>>>          unsigned long pmdv;
>>>>>
>>>>>          pmdv = (pfn << PAGE_SHIFT) & PTE_RPN_MASK;
>>>>>
>>>>>          return __pmd_mkhuge(pmd_set_protbits(__pmd(pmdv), pgprot));
>>>>> }
>>>>>
>>>>> It seems like on powerpc, where pfn_pmd() makes a huge page but which
>>>>> is not supported on 4K hash config thus triggering the BUG(). But all
>>>>> pfn_pmd() call sites inside the debug_vm_pgtable() test are protected
>>>>> with CONFIG_TRANSPARENT_HUGEPAGE. IIUC unlike powerpc, pfn_pmd() does
>>>>> not directly make a huge page on other platforms.
>>>>>
>>>>> Looking at arch/powerpc/include/asm/book3s/64/hash-4k.h, all relevant
>>>>> THP helpers has BUG() or 0 which indicates THP might not be supported
>>>>> on 4K page size hash config ?
>>>>>
>>>>> But looking at arch/powerpc/platforms/Kconfig.cputype, it seems like
>>>>> HAVE_ARCH_TRANSPARENT_HUGEPAGE is invariably selected on PPC_BOOK3S_64
>>>>> platforms which I assume includes 4K page size hash config as well.
>>>>>
>>>>> Is THP some how getting enabled on this 4K page size hash config where
>>>>> it should not be (thus triggering the BUG) ? OR am I missing something
>>>>> here.
>>>>>
>>>>
>>>
>>> We should put those  pfn_pmd()  and pfn_pud() after
>>>
>>>     if (!has_transparent_hugepage())
>>>         return;
>>
>> The following patch has been lightly tested on arm64 and x86 platforms.
>> Could you please verify if this solves the problem on powerpc 4K hash
>> config ? Thank you.
> 
> Tested the patch with the below additional change.
> 
> modified   mm/debug_vm_pgtable.c
> @@ -186,12 +186,13 @@ static void __init pmd_advanced_tests(struct mm_struct *mm,
>  				      unsigned long pfn, unsigned long vaddr,
>  				      pgprot_t prot, pgtable_t pgtable)
>  {
> -	pmd_t pmd = pfn_pmd(pfn, prot);
> +	pmd_t pmd;
>  
>  	if (!has_transparent_hugepage())
>  		return;
>  
>  	pr_debug("Validating PMD advanced\n");
> +	pmd = pfn_pmd(pfn, prot);
>  	/* Align the address wrt HPAGE_PMD_SIZE */
>  	vaddr = (vaddr & HPAGE_PMD_MASK) + HPAGE_PMD_SIZE;
>  
> @@ -334,12 +335,13 @@ static void __init pud_advanced_tests(struct mm_struct *mm,
>  				      unsigned long pfn, unsigned long vaddr,
>  				      pgprot_t prot)
>  {
> -	pud_t pud = pfn_pud(pfn, prot);
> +	pud_t pud;
>  
>  	if (!has_transparent_hugepage())
>  		return;
>  
>  	pr_debug("Validating PUD advanced\n");
> +	pud = pfn_pud(pfn, prot);
>  	/* Align the address wrt HPAGE_PUD_SIZE */
>  	vaddr = (vaddr & HPAGE_PUD_MASK) + HPAGE_PUD_SIZE;
>  

Right. But this change is already on the linux-next (20210514) via another
commit ae7920ce9e9bb ("mm/debug_vm_pgtable: remove redundant pfn_{pmd/pte}()
and fix one comment mistake") which got queued recently. Anyways, now that
we have verified the fix, will send it out against linux-next soon.

^ permalink raw reply

* Re: [PATCH 14/14] powerpc/64s: use the same default PPR for user and kernel
From: Christophe Leroy @ 2021-05-17 14:09 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev
In-Reply-To: <20210315220402.260594-15-npiggin@gmail.com>



Le 15/03/2021 à 23:04, Nicholas Piggin a écrit :
> Change the default PPR to userspace to 4 (medium), matching the
> normal kernel PPR.
> 
> This allows system calls and user interrupts to avoid setting PPR on
> entry and exit, providing a significant speedup.
> 
> This is a change to the user environment. The problem with changing
> the kernel to match userspace at 3 (medium-low), is that userspace
> can then boost priority above the kernel which is also undesirable.
> 
> glibc does not seem to change PPR anywhere, so the decision is to
> go with this.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   arch/powerpc/include/asm/interrupt.h |  2 ++
>   arch/powerpc/include/asm/processor.h |  4 ++--
>   arch/powerpc/kernel/exceptions-64s.S |  3 ---
>   arch/powerpc/kernel/interrupt.c      | 33 ++++++++++++++++++++++++++++
>   arch/powerpc/kernel/interrupt_64.S   | 17 --------------
>   5 files changed, 37 insertions(+), 22 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
> index d6d54bbcba2f..293e6be9fd71 100644
> --- a/arch/powerpc/include/asm/interrupt.h
> +++ b/arch/powerpc/include/asm/interrupt.h
> @@ -57,6 +57,8 @@ static inline void interrupt_enter_prepare(struct pt_regs *regs, struct interrup
>   #ifdef CONFIG_PPC64
>   	bool trace_enable = false;
>   
> +	if (unlikely(regs->ppr != DEFAULT_PPR))
> +		mtspr(SPRN_PPR, DEFAULT_PPR);
>   	if (IS_ENABLED(CONFIG_TRACE_IRQFLAGS)) {
>   		if (irq_soft_mask_set_return(IRQS_DISABLED) == IRQS_ENABLED)
>   			trace_enable = true;
> diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
> index cb1edf21a82e..5ff589042103 100644
> --- a/arch/powerpc/include/asm/processor.h
> +++ b/arch/powerpc/include/asm/processor.h
> @@ -27,8 +27,8 @@
>   #endif
>   
>   #ifdef CONFIG_PPC64
> -/* Default SMT priority is set to 3. Use 11- 13bits to save priority. */
> -#define PPR_PRIORITY 3
> +/* Default SMT priority is set to 4. Use 11- 13bits to save priority. */
> +#define PPR_PRIORITY 4
>   #ifdef __ASSEMBLY__
>   #define DEFAULT_PPR (PPR_PRIORITY << 50)
>   #else
> diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
> index 75cee7cdf887..0d40614d13e0 100644
> --- a/arch/powerpc/kernel/exceptions-64s.S
> +++ b/arch/powerpc/kernel/exceptions-64s.S
> @@ -367,7 +367,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
>   BEGIN_FTR_SECTION
>   	mfspr	r9,SPRN_PPR
>   END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
> -	HMT_MEDIUM
>   	std	r10,IAREA+EX_R10(r13)		/* save r10 - r12 */
>   BEGIN_FTR_SECTION
>   	mfspr	r10,SPRN_CFAR
> @@ -1962,8 +1961,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_REAL_LE)
>   	mfspr	r11,SPRN_SRR0
>   	mfspr	r12,SPRN_SRR1
>   
> -	HMT_MEDIUM
> -
>   	.if ! \virt
>   	__LOAD_HANDLER(r10, system_call_common_real)
>   	mtctr	r10
> diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
> index 09cf699d0e2e..a6e0595da0dd 100644
> --- a/arch/powerpc/kernel/interrupt.c
> +++ b/arch/powerpc/kernel/interrupt.c
> @@ -40,6 +40,11 @@ notrace long system_call_exception(long r3, long r4, long r5,
>   
>   	regs->orig_gpr3 = r3;
>   
> +#ifdef CONFIG_PPC_BOOK3S_64
> +	if (unlikely(regs->ppr != DEFAULT_PPR))
> +		mtspr(SPRN_PPR, DEFAULT_PPR);
> +#endif

Can you have some helper functions to do this instead of those 4 lines #ifdefed blocks all over the 
place ?

Something like

#ifdef CONFIG_PPC_BOOK3S_64
static inline void set_ppr_regs(struct pt_regs *regs)
{
	if (unlikely(regs->ppr != DEFAULT_PPR))
		mtspr(SPRN_PPR, regs->ppr);
}

static inline void set_ppr_default(struct pt_regs *regs)
{
	if (unlikely(regs->ppr != DEFAULT_PPR))
		mtspr(SPRN_PPR, DEFAULT_PPR);
}
#else
static inline void set_ppr_regs(struct pt_regs *regs) { }
static inline void set_ppr_default(struct pt_regs *regs) { }
#endif

> +
>   	if (IS_ENABLED(CONFIG_PPC_IRQ_SOFT_MASK_DEBUG))
>   		BUG_ON(irq_soft_mask_return() != IRQS_ALL_DISABLED);
>   
> @@ -237,6 +242,11 @@ notrace unsigned long syscall_exit_prepare_main(unsigned long r3,
>   
>   	account_cpu_user_exit();
>   
> +#ifdef CONFIG_PPC_BOOK3S_64
> +	if (unlikely(regs->ppr != DEFAULT_PPR))
> +		mtspr(SPRN_PPR, regs->ppr);
> +#endif
> +
>   #ifndef CONFIG_PPC_BOOK3E_64 /* BOOK3E not using this */
>   	/*
>   	 * We do this at the end so that we do context switch with KERNEL AMR
> @@ -315,6 +325,11 @@ notrace unsigned long syscall_exit_restart(unsigned long r3, struct pt_regs *reg
>   	 */
>   	hard_irq_disable();
>   
> +#ifdef CONFIG_PPC_BOOK3S_64
> +	if (unlikely(regs->ppr != DEFAULT_PPR))
> +		mtspr(SPRN_PPR, DEFAULT_PPR);
> +#endif
> +
>   	trace_hardirqs_off();
>   	user_exit_irqoff();
>   	account_cpu_user_entry();
> @@ -398,6 +413,11 @@ notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs)
>   
>   	account_cpu_user_exit();
>   
> +#ifdef CONFIG_PPC_BOOK3S_64
> +	if (unlikely(regs->ppr != DEFAULT_PPR))
> +		mtspr(SPRN_PPR, regs->ppr);
> +#endif
> +
>   	/*
>   	 * We do this at the end so that we do context switch with KERNEL AMR
>   	 */
> @@ -489,6 +509,11 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs)
>   	local_paca->tm_scratch = regs->msr;
>   #endif
>   
> +#ifdef CONFIG_PPC_BOOK3S_64
> +	if (unlikely(regs->ppr != DEFAULT_PPR))
> +		mtspr(SPRN_PPR, regs->ppr);
> +#endif
> +
>   	/*
>   	 * Don't want to mfspr(SPRN_AMR) here, because this comes after mtmsr,
>   	 * which would cause Read-After-Write stalls. Hence, we take the AMR
> @@ -505,6 +530,10 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs)
>   notrace unsigned long interrupt_exit_user_restart(struct pt_regs *regs)
>   {
>   	hard_irq_disable();
> +#ifdef CONFIG_PPC_BOOK3S_64
> +	if (unlikely(regs->ppr != DEFAULT_PPR))
> +		mtspr(SPRN_PPR, DEFAULT_PPR);
> +#endif
>   
>   	trace_hardirqs_off();
>   	user_exit_irqoff();
> @@ -523,6 +552,10 @@ notrace unsigned long interrupt_exit_user_restart(struct pt_regs *regs)
>   notrace unsigned long interrupt_exit_kernel_restart(struct pt_regs *regs)
>   {
>   	hard_irq_disable();
> +#ifdef CONFIG_PPC_BOOK3S_64
> +	if (unlikely(regs->ppr != DEFAULT_PPR))
> +		mtspr(SPRN_PPR, DEFAULT_PPR);
> +#endif
>   
>   #ifndef CONFIG_PPC_BOOK3E_64
>   	set_kuap(AMR_KUAP_BLOCKED);
> diff --git a/arch/powerpc/kernel/interrupt_64.S b/arch/powerpc/kernel/interrupt_64.S
> index eef61800f734..53fc446dcbeb 100644
> --- a/arch/powerpc/kernel/interrupt_64.S
> +++ b/arch/powerpc/kernel/interrupt_64.S
> @@ -99,10 +99,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_TM)
>   	ld	r11,exception_marker@toc(r2)
>   	std	r11,-16(r10)		/* "regshere" marker */
>   
> -BEGIN_FTR_SECTION
> -	HMT_MEDIUM
> -END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
> -
>   	ENTER_KERNEL_SECURITY_FALLBACK
>   
>   	/*
> @@ -142,10 +138,6 @@ BEGIN_FTR_SECTION
>   	stdcx.	r0,0,r1			/* to clear the reservation */
>   END_FTR_SECTION_IFCLR(CPU_FTR_STCX_CHECKS_ADDRESS)
>   
> -BEGIN_FTR_SECTION
> -	HMT_MEDIUM_LOW
> -END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
> -
>   	cmpdi	r3,0
>   	bne	.Lsyscall_vectored_\name\()_restore_regs
>   
> @@ -377,10 +369,6 @@ END_FTR_SECTION_IFCLR(CPU_FTR_STCX_CHECKS_ADDRESS)
>   	mtspr	SPRN_XER,r0
>   .Lsyscall_restore_regs_cont:
>   
> -BEGIN_FTR_SECTION
> -	HMT_MEDIUM_LOW
> -END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
> -
>   	/*
>   	 * We don't need to restore AMR on the way back to userspace for KUAP.
>   	 * The value of AMR only matters while we're in the kernel.
> @@ -533,11 +521,6 @@ _ASM_NOKPROBE_SYMBOL(interrupt_return_\srr\())
>   	tdnei	r4,IRQS_ENABLED
>   
>   #ifdef CONFIG_PPC_BOOK3S
> -BEGIN_FTR_SECTION
> -	ld	r10,_PPR(r1)
> -	mtspr	SPRN_PPR,r10
> -END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
> -
>   	.ifc \srr,srr
>   	lbz	r4,PACASRR_VALID(r13)
>   	.else
> 

^ permalink raw reply

* [PATCH] powerpc/powernv: Fix machine check reporting of async store errors
From: Nicholas Piggin @ 2021-05-17 14:03 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Mahesh Salgaonkar, Nicholas Piggin

POWER9 and POWER10 asynchronous machine checks due to stores have their
cause reported in SRR1 but SRR1[42] is set, which in other cases
indicates DSISR cause.

Check for these cases and clear SRR1[42], so the cause matching uses
the i-side (SRR1) table.

Cc: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
Fixes: 7b9f71f974 ("powerpc/64s: POWER9 machine check handler")
Fixes: 201220bb0e ("powerpc/powernv: Machine check handler for POWER10")
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kernel/mce_power.c | 48 +++++++++++++++++++++++++++------
 1 file changed, 40 insertions(+), 8 deletions(-)

diff --git a/arch/powerpc/kernel/mce_power.c b/arch/powerpc/kernel/mce_power.c
index 667104d4c455..2fff886c549d 100644
--- a/arch/powerpc/kernel/mce_power.c
+++ b/arch/powerpc/kernel/mce_power.c
@@ -481,12 +481,11 @@ static int mce_find_instr_ea_and_phys(struct pt_regs *regs, uint64_t *addr,
 	return -1;
 }
 
-static int mce_handle_ierror(struct pt_regs *regs,
+static int mce_handle_ierror(struct pt_regs *regs, unsigned long srr1,
 		const struct mce_ierror_table table[],
 		struct mce_error_info *mce_err, uint64_t *addr,
 		uint64_t *phys_addr)
 {
-	uint64_t srr1 = regs->msr;
 	int handled = 0;
 	int i;
 
@@ -695,19 +694,19 @@ static long mce_handle_ue_error(struct pt_regs *regs,
 }
 
 static long mce_handle_error(struct pt_regs *regs,
+		unsigned long srr1,
 		const struct mce_derror_table dtable[],
 		const struct mce_ierror_table itable[])
 {
 	struct mce_error_info mce_err = { 0 };
 	uint64_t addr, phys_addr = ULONG_MAX;
-	uint64_t srr1 = regs->msr;
 	long handled;
 
 	if (SRR1_MC_LOADSTORE(srr1))
 		handled = mce_handle_derror(regs, dtable, &mce_err, &addr,
 				&phys_addr);
 	else
-		handled = mce_handle_ierror(regs, itable, &mce_err, &addr,
+		handled = mce_handle_ierror(regs, srr1, itable, &mce_err, &addr,
 				&phys_addr);
 
 	if (!handled && mce_err.error_type == MCE_ERROR_TYPE_UE)
@@ -723,16 +722,20 @@ long __machine_check_early_realmode_p7(struct pt_regs *regs)
 	/* P7 DD1 leaves top bits of DSISR undefined */
 	regs->dsisr &= 0x0000ffff;
 
-	return mce_handle_error(regs, mce_p7_derror_table, mce_p7_ierror_table);
+	return mce_handle_error(regs, regs->msr,
+			mce_p7_derror_table, mce_p7_ierror_table);
 }
 
 long __machine_check_early_realmode_p8(struct pt_regs *regs)
 {
-	return mce_handle_error(regs, mce_p8_derror_table, mce_p8_ierror_table);
+	return mce_handle_error(regs, regs->msr,
+			mce_p8_derror_table, mce_p8_ierror_table);
 }
 
 long __machine_check_early_realmode_p9(struct pt_regs *regs)
 {
+	unsigned long srr1 = regs->msr;
+
 	/*
 	 * On POWER9 DD2.1 and below, it's possible to get a machine check
 	 * caused by a paste instruction where only DSISR bit 25 is set. This
@@ -746,10 +749,39 @@ long __machine_check_early_realmode_p9(struct pt_regs *regs)
 	if (SRR1_MC_LOADSTORE(regs->msr) && regs->dsisr == 0x02000000)
 		return 1;
 
-	return mce_handle_error(regs, mce_p9_derror_table, mce_p9_ierror_table);
+	/*
+	 * Async machine check due to bad real address from store or foreign
+	 * link time out comes with the load/store bit (PPC bit 42) set in
+	 * SRR1, but the cause comes in SRR1 not DSISR. Clear bit 42 so we're
+	 * directed to the ierror table so it will find the cause (which
+	 * describes it correctly as a store error).
+	 */
+	if (SRR1_MC_LOADSTORE(srr1) &&
+			((srr1 & 0x081c0000) == 0x08140000 ||
+			 (srr1 & 0x081c0000) == 0x08180000)) {
+		srr1 &= ~PPC_BIT(42);
+	}
+
+	return mce_handle_error(regs, srr1,
+			mce_p9_derror_table, mce_p9_ierror_table);
 }
 
 long __machine_check_early_realmode_p10(struct pt_regs *regs)
 {
-	return mce_handle_error(regs, mce_p10_derror_table, mce_p10_ierror_table);
+	unsigned long srr1 = regs->msr;
+
+	/*
+	 * Async machine check due to bad real address from store comes with
+	 * the load/store bit (PPC bit 42) set in SRR1, but the cause comes in
+	 * SRR1 not DSISR. Clear bit 42 so we're directed to the ierror table
+	 * so it will find the cause (which describes it correctly as a store
+	 * error).
+	 */
+	if (SRR1_MC_LOADSTORE(srr1) &&
+			(srr1 & 0x081c0000) == 0x08140000) {
+		srr1 &= ~PPC_BIT(42);
+	}
+
+	return mce_handle_error(regs, srr1,
+			mce_p10_derror_table, mce_p10_ierror_table);
 }
-- 
2.23.0


^ permalink raw reply related

* Re: [PATCH 1/2] powerpc/interrupt: Refactor interrupt_exit_user_prepare() and syscall_exit_prepare()
From: Christophe Leroy @ 2021-05-17 13:57 UTC (permalink / raw)
  To: Nicholas Piggin, Benjamin Herrenschmidt, Michael Ellerman,
	Paul Mackerras
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1621237386.33q9uyrpc3.astroid@bobo.none>



Le 17/05/2021 à 09:44, Nicholas Piggin a écrit :
> Excerpts from Christophe Leroy's message of May 14, 2021 6:28 pm:
>> Last part of interrupt_exit_user_prepare() and syscall_exit_prepare()
>> are identical.
>>
>> Create a __interrupt_exit_user_prepare() function that is called by
>> both.
>>
>> Note that it replaces a local_irq_save(flags) by local_irq_disable().
>> This is similar because the flags are never used. On ppc 8xx it is
>> more efficient because it doesn't require reading MSR.
> 
> Can these cleanups go after my interrupt performance improvements?
> I posted them for last series but were dropped due to crashes without
> time to resubmit. I'm working on them again now.
> 

Euh ... ok why not, but at the time being interrupt_exit_user_prepare() and syscall_exit_prepare() 
are very similar. Which makes sense because both of them are returning from kernel to user so they 
are to do the same preparation.

If you are doing the same changes to both of them, maybe it is worst including this refactor at the 
begining of your series. Or are you making them diverge with that series ?

Christophe

^ permalink raw reply

* Re: [PATCH v5 5/9] powerpc/mm/book3s64: Update tlb flush routines to take a page walk cache flush argument
From: Aneesh Kumar K.V @ 2021-05-17 13:55 UTC (permalink / raw)
  To: Guenter Roeck; +Cc: npiggin, linux-mm, kaleshsingh, joel, akpm, linuxppc-dev
In-Reply-To: <d830fce9-c00a-e879-4115-94a2346a806f@roeck-us.net>

Guenter Roeck <linux@roeck-us.net> writes:

> On 5/17/21 1:40 AM, Aneesh Kumar K.V wrote:
>> On 5/15/21 10:05 PM, Guenter Roeck wrote:
>>> On Thu, Apr 22, 2021 at 11:13:19AM +0530, Aneesh Kumar K.V wrote:

...

>>>   extern void radix__local_flush_all_mm(struct mm_struct *mm);
>>>> diff --git a/arch/powerpc/include/asm/book3s/64/tlbflush.h b/arch/powerpc/include/asm/book3s/64/tlbflush.h
>>>> index 215973b4cb26..f9f8a3a264f7 100644
>>>> --- a/arch/powerpc/include/asm/book3s/64/tlbflush.h
>>>> +++ b/arch/powerpc/include/asm/book3s/64/tlbflush.h
>>>> @@ -45,13 +45,30 @@ static inline void tlbiel_all_lpid(bool radix)
>>>>           hash__tlbiel_all(TLB_INVAL_SCOPE_LPID);
>>>>   }
>>>> +static inline void flush_pmd_tlb_pwc_range(struct vm_area_struct *vma,
>>>                   ^^^^
>>>> +                       unsigned long start,
>>>> +                       unsigned long end,
>>>> +                       bool flush_pwc)
>>>> +{
>>>> +    if (radix_enabled())
>>>> +        return radix__flush_pmd_tlb_range(vma, start, end, flush_pwc);
>>>> +    return hash__flush_tlb_range(vma, start, end);
>>>          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>>
>>>> +}
>> 
>> In this specific case we won't have  build errors because,
>> 
>> static inline void hash__flush_tlb_range(struct vm_area_struct *vma,
>>                       unsigned long start, unsigned long end)
>> {
>> 
>
> Sorry, you completely lost me.
>
> Building parisc:allnoconfig ... failed
> --------------
> Error log:
> In file included from arch/parisc/include/asm/cacheflush.h:7,
>                   from include/linux/highmem.h:12,
>                   from include/linux/pagemap.h:11,
>                   from include/linux/ksm.h:13,
>                   from mm/mremap.c:14:
> mm/mremap.c: In function 'flush_pte_tlb_pwc_range':
> arch/parisc/include/asm/tlbflush.h:20:2: error: 'return' with a value, in function returning void

As replied here
https://lore.kernel.org/mm-commits/8eedb441-a612-1ec8-8bf7-b40184de9f6f@linux.ibm.com/

That was the generic header change in the patch. I was commenting about the
ppc64 specific change causing build failures.

-aneesh


^ permalink raw reply

* Re: Fwd: [Bug 213069] New: kernel BUG at arch/powerpc/include/asm/book3s/64/hash-4k.h:147! Oops: Exception in kernel mode, sig: 5 [#1]
From: Aneesh Kumar K.V @ 2021-05-17 13:52 UTC (permalink / raw)
  To: Anshuman Khandual, Christophe Leroy,
	linuxppc-dev@lists.ozlabs.org
In-Reply-To: <6fcdf4f2-9bed-4b0f-8ed8-74cb25484ea6@arm.com>

Anshuman Khandual <anshuman.khandual@arm.com> writes:

> On 5/17/21 11:25 AM, Aneesh Kumar K.V wrote:
>> On 5/17/21 11:17 AM, Christophe Leroy wrote:
>>> +aneesh
>>> +linuxppc-dev list
>>>
>>> Le 17/05/2021 à 07:44, Anshuman Khandual a écrit :
>>>> Hello Christophe,
>>>>
>>>> DEBUG_VM_PGTABLE has now been re-enabled on powerpc recently ? was not
>>>> aware about this. From the error log, it failed explicitly on 4K page
>>>> size hash config.
>>>>
>>>> static inline pmd_t hash__pmd_mkhuge(pmd_t pmd)
>>>> {
>>>>          BUG();        ------> Failed
>>>>          return pmd;
>>>> }
>>>>
>>>> static inline pmd_t __pmd_mkhuge(pmd_t pmd)
>>>> {
>>>>          if (radix_enabled())
>>>>                  return radix__pmd_mkhuge(pmd);
>>>>          return hash__pmd_mkhuge(pmd);
>>>> }
>>>>
>>>> pmd_t pfn_pmd(unsigned long pfn, pgprot_t pgprot)
>>>> {
>>>>          unsigned long pmdv;
>>>>
>>>>          pmdv = (pfn << PAGE_SHIFT) & PTE_RPN_MASK;
>>>>
>>>>          return __pmd_mkhuge(pmd_set_protbits(__pmd(pmdv), pgprot));
>>>> }
>>>>
>>>> It seems like on powerpc, where pfn_pmd() makes a huge page but which
>>>> is not supported on 4K hash config thus triggering the BUG(). But all
>>>> pfn_pmd() call sites inside the debug_vm_pgtable() test are protected
>>>> with CONFIG_TRANSPARENT_HUGEPAGE. IIUC unlike powerpc, pfn_pmd() does
>>>> not directly make a huge page on other platforms.
>>>>
>>>> Looking at arch/powerpc/include/asm/book3s/64/hash-4k.h, all relevant
>>>> THP helpers has BUG() or 0 which indicates THP might not be supported
>>>> on 4K page size hash config ?
>>>>
>>>> But looking at arch/powerpc/platforms/Kconfig.cputype, it seems like
>>>> HAVE_ARCH_TRANSPARENT_HUGEPAGE is invariably selected on PPC_BOOK3S_64
>>>> platforms which I assume includes 4K page size hash config as well.
>>>>
>>>> Is THP some how getting enabled on this 4K page size hash config where
>>>> it should not be (thus triggering the BUG) ? OR am I missing something
>>>> here.
>>>>
>>>
>> 
>> We should put those  pfn_pmd()  and pfn_pud() after
>> 
>>     if (!has_transparent_hugepage())
>>         return;
>
> The following patch has been lightly tested on arm64 and x86 platforms.
> Could you please verify if this solves the problem on powerpc 4K hash
> config ? Thank you.

Tested the patch with the below additional change.

modified   mm/debug_vm_pgtable.c
@@ -186,12 +186,13 @@ static void __init pmd_advanced_tests(struct mm_struct *mm,
 				      unsigned long pfn, unsigned long vaddr,
 				      pgprot_t prot, pgtable_t pgtable)
 {
-	pmd_t pmd = pfn_pmd(pfn, prot);
+	pmd_t pmd;
 
 	if (!has_transparent_hugepage())
 		return;
 
 	pr_debug("Validating PMD advanced\n");
+	pmd = pfn_pmd(pfn, prot);
 	/* Align the address wrt HPAGE_PMD_SIZE */
 	vaddr = (vaddr & HPAGE_PMD_MASK) + HPAGE_PMD_SIZE;
 
@@ -334,12 +335,13 @@ static void __init pud_advanced_tests(struct mm_struct *mm,
 				      unsigned long pfn, unsigned long vaddr,
 				      pgprot_t prot)
 {
-	pud_t pud = pfn_pud(pfn, prot);
+	pud_t pud;
 
 	if (!has_transparent_hugepage())
 		return;
 
 	pr_debug("Validating PUD advanced\n");
+	pud = pfn_pud(pfn, prot);
 	/* Align the address wrt HPAGE_PUD_SIZE */
 	vaddr = (vaddr & HPAGE_PUD_MASK) + HPAGE_PUD_SIZE;
 



-aneesh

^ permalink raw reply

* Re: [PATCH 01/14] powerpc: remove interrupt exit helpers unused argument
From: Christophe Leroy @ 2021-05-17 13:49 UTC (permalink / raw)
  To: Nicholas Piggin, linuxppc-dev
In-Reply-To: <20210315220402.260594-2-npiggin@gmail.com>



Le 15/03/2021 à 23:03, Nicholas Piggin a écrit :
> The msr argument is not used, remove it.

And why not use it instead of re-reading regs->msr ?

> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   arch/powerpc/include/asm/asm-prototypes.h | 4 ++--
>   arch/powerpc/kernel/interrupt.c           | 4 ++--
>   2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/asm-prototypes.h b/arch/powerpc/include/asm/asm-prototypes.h
> index 1c7b75834e04..95492655462e 100644
> --- a/arch/powerpc/include/asm/asm-prototypes.h
> +++ b/arch/powerpc/include/asm/asm-prototypes.h
> @@ -71,8 +71,8 @@ void __init machine_init(u64 dt_ptr);
>   #endif
>   long system_call_exception(long r3, long r4, long r5, long r6, long r7, long r8, unsigned long r0, struct pt_regs *regs);
>   notrace unsigned long syscall_exit_prepare(unsigned long r3, struct pt_regs *regs, long scv);
> -notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs, unsigned long msr);
> -notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsigned long msr);
> +notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs);
> +notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs);
>   
>   long ppc_fadvise64_64(int fd, int advice, u32 offset_high, u32 offset_low,
>   		      u32 len_high, u32 len_low);
> diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
> index 96ca27ef68ae..efeeefe6ee8f 100644
> --- a/arch/powerpc/kernel/interrupt.c
> +++ b/arch/powerpc/kernel/interrupt.c
> @@ -359,7 +359,7 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
>   	return ret;
>   }
>   
> -notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs, unsigned long msr)
> +notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs)
>   {
>   	unsigned long ti_flags;
>   	unsigned long flags;
> @@ -443,7 +443,7 @@ notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs, unsigned
>   
>   void preempt_schedule_irq(void);
>   
> -notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsigned long msr)
> +notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs)
>   {
>   	unsigned long flags;
>   	unsigned long ret = 0;
> 

^ permalink raw reply

* Re: [PATCH] watchdog: Remove MV64x60 watchdog driver
From: Guenter Roeck @ 2021-05-17 13:39 UTC (permalink / raw)
  To: Michael Ellerman, Christophe Leroy, Wim Van Sebroeck,
	Sebastian Hesselbarth
  Cc: netdev, linuxppc-dev, linux-kernel, linux-watchdog
In-Reply-To: <87im3hk3t2.fsf@mpe.ellerman.id.au>

On 5/17/21 4:17 AM, Michael Ellerman wrote:
> Guenter Roeck <linux@roeck-us.net> writes:
>> On 3/18/21 10:25 AM, Christophe Leroy wrote:
>>> Commit 92c8c16f3457 ("powerpc/embedded6xx: Remove C2K board support")
>>> removed the last selector of CONFIG_MV64X60.
>>>
>>> Therefore CONFIG_MV64X60_WDT cannot be selected anymore and
>>> can be removed.
>>>
>>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>>
>> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
>>
>>> ---
>>>   drivers/watchdog/Kconfig       |   4 -
>>>   drivers/watchdog/Makefile      |   1 -
>>>   drivers/watchdog/mv64x60_wdt.c | 324 ---------------------------------
>>>   include/linux/mv643xx.h        |   8 -
>>>   4 files changed, 337 deletions(-)
>>>   delete mode 100644 drivers/watchdog/mv64x60_wdt.c
> 
> I assumed this would go via the watchdog tree, but seems like I
> misinterpreted.
> 

Wim didn't send a pull request this time around.

Guenter

> Should I take this via the powerpc tree for v5.14 ?
> 
> cheers
> 


^ permalink raw reply

* Re: [PATCH v5 5/9] powerpc/mm/book3s64: Update tlb flush routines to take a page walk cache flush argument
From: Guenter Roeck @ 2021-05-17 13:38 UTC (permalink / raw)
  To: Aneesh Kumar K.V; +Cc: npiggin, linux-mm, kaleshsingh, joel, akpm, linuxppc-dev
In-Reply-To: <e0eba73a-c2df-71c3-e03d-d4074d908fca@linux.ibm.com>

On 5/17/21 1:40 AM, Aneesh Kumar K.V wrote:
> On 5/15/21 10:05 PM, Guenter Roeck wrote:
>> On Thu, Apr 22, 2021 at 11:13:19AM +0530, Aneesh Kumar K.V wrote:
>>> No functional change in this patch
>>>
>>> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>>> ---
>>>   .../include/asm/book3s/64/tlbflush-radix.h    | 19 +++++++-----
>>>   arch/powerpc/include/asm/book3s/64/tlbflush.h | 23 ++++++++++++---
>>>   arch/powerpc/mm/book3s64/radix_hugetlbpage.c  |  4 +--
>>>   arch/powerpc/mm/book3s64/radix_tlb.c          | 29 +++++++------------
>>>   4 files changed, 42 insertions(+), 33 deletions(-)
>>>
>>> diff --git a/arch/powerpc/include/asm/book3s/64/tlbflush-radix.h b/arch/powerpc/include/asm/book3s/64/tlbflush-radix.h
>>> index 8b33601cdb9d..171441a43b35 100644
>>> --- a/arch/powerpc/include/asm/book3s/64/tlbflush-radix.h
>>> +++ b/arch/powerpc/include/asm/book3s/64/tlbflush-radix.h
>>> @@ -56,15 +56,18 @@ static inline void radix__flush_all_lpid_guest(unsigned int lpid)
>>>   }
>>>   #endif
>>> -extern void radix__flush_hugetlb_tlb_range(struct vm_area_struct *vma,
>>> -                       unsigned long start, unsigned long end);
>>> -extern void radix__flush_tlb_range_psize(struct mm_struct *mm, unsigned long start,
>>> -                     unsigned long end, int psize);
>>> -extern void radix__flush_pmd_tlb_range(struct vm_area_struct *vma,
>>> -                       unsigned long start, unsigned long end);
>>> -extern void radix__flush_tlb_range(struct vm_area_struct *vma, unsigned long start,
>>> +void radix__flush_hugetlb_tlb_range(struct vm_area_struct *vma,
>>> +                    unsigned long start, unsigned long end,
>>> +                    bool flush_pwc);
>>> +void radix__flush_pmd_tlb_range(struct vm_area_struct *vma,
>>> +                unsigned long start, unsigned long end,
>>> +                bool flush_pwc);
>>> +void radix__flush_tlb_pwc_range_psize(struct mm_struct *mm, unsigned long start,
>>> +                      unsigned long end, int psize, bool flush_pwc);
>>> +void radix__flush_tlb_range(struct vm_area_struct *vma, unsigned long start,
>>>                   unsigned long end);
>>> -extern void radix__flush_tlb_kernel_range(unsigned long start, unsigned long end);
>>> +void radix__flush_tlb_kernel_range(unsigned long start, unsigned long end);
>>> +
>>>   extern void radix__local_flush_tlb_mm(struct mm_struct *mm);
>>>   extern void radix__local_flush_all_mm(struct mm_struct *mm);
>>> diff --git a/arch/powerpc/include/asm/book3s/64/tlbflush.h b/arch/powerpc/include/asm/book3s/64/tlbflush.h
>>> index 215973b4cb26..f9f8a3a264f7 100644
>>> --- a/arch/powerpc/include/asm/book3s/64/tlbflush.h
>>> +++ b/arch/powerpc/include/asm/book3s/64/tlbflush.h
>>> @@ -45,13 +45,30 @@ static inline void tlbiel_all_lpid(bool radix)
>>>           hash__tlbiel_all(TLB_INVAL_SCOPE_LPID);
>>>   }
>>> +static inline void flush_pmd_tlb_pwc_range(struct vm_area_struct *vma,
>>                   ^^^^
>>> +                       unsigned long start,
>>> +                       unsigned long end,
>>> +                       bool flush_pwc)
>>> +{
>>> +    if (radix_enabled())
>>> +        return radix__flush_pmd_tlb_range(vma, start, end, flush_pwc);
>>> +    return hash__flush_tlb_range(vma, start, end);
>>          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>
>>> +}
> 
> In this specific case we won't have  build errors because,
> 
> static inline void hash__flush_tlb_range(struct vm_area_struct *vma,
>                       unsigned long start, unsigned long end)
> {
> 

Sorry, you completely lost me.

Building parisc:allnoconfig ... failed
--------------
Error log:
In file included from arch/parisc/include/asm/cacheflush.h:7,
                  from include/linux/highmem.h:12,
                  from include/linux/pagemap.h:11,
                  from include/linux/ksm.h:13,
                  from mm/mremap.c:14:
mm/mremap.c: In function 'flush_pte_tlb_pwc_range':
arch/parisc/include/asm/tlbflush.h:20:2: error: 'return' with a value, in function returning void

Guenter

> 
> But I agree the below is better to read.
> 
> static inline void flush_pmd_tlb_pwc_range(struct vm_area_struct *vma,
>                         unsigned long start,
>                         unsigned long end,
>                         bool flush_pwc)
> {
>      if (radix_enabled())
>          radix__flush_pmd_tlb_range(vma, start, end, flush_pwc);
>      else
>          hash__flush_tlb_range(vma, start, end);
>      return
> }
> 
>>
>>>   #define __HAVE_ARCH_FLUSH_PMD_TLB_RANGE
>>>   static inline void flush_pmd_tlb_range(struct vm_area_struct *vma,
>>                   ^^^^
>>>                          unsigned long start, unsigned long end)
>>> +{
>>> +    return flush_pmd_tlb_pwc_range(vma, start, end, false);
>>          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>
>> Doesn't that cause build warnings/errors all over the place ?
>>
>> Guenter
>>
> 
> 
> -aneesh


^ permalink raw reply

* Re: [PATCH v2 2/2] powerpc/sstep: Add tests for setb instruction
From: Naveen N. Rao @ 2021-05-17 13:34 UTC (permalink / raw)
  To: linuxppc-dev, Sathvika Vasireddy; +Cc: dja
In-Reply-To: <b05b61ccb5f10279d46fed490796f32ea2ccc270.1620727160.git.sathvika@linux.vnet.ibm.com>

Sathvika Vasireddy wrote:
> This adds selftests for setb instruction.
> 
> Signed-off-by: Sathvika Vasireddy <sathvika@linux.vnet.ibm.com>
> ---
>  arch/powerpc/include/asm/ppc-opcode.h |  1 +
>  arch/powerpc/lib/test_emulate_step.c  | 29 +++++++++++++++++++++++++++++
>  2 files changed, 30 insertions(+)

Tested-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>


> 
> diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
> index ac41776661e9..927551dd870b 100644
> --- a/arch/powerpc/include/asm/ppc-opcode.h
> +++ b/arch/powerpc/include/asm/ppc-opcode.h
> @@ -245,6 +245,7 @@
>  #define PPC_INST_STRING			0x7c00042a
>  #define PPC_INST_STRING_MASK		0xfc0007fe
>  #define PPC_INST_STRING_GEN_MASK	0xfc00067e
> +#define PPC_INST_SETB			0x7c000100
>  #define PPC_INST_STSWI			0x7c0005aa
>  #define PPC_INST_STSWX			0x7c00052a
>  #define PPC_INST_TRECHKPT		0x7c0007dd
> diff --git a/arch/powerpc/lib/test_emulate_step.c b/arch/powerpc/lib/test_emulate_step.c
> index 783d1b85ecfe..a0a52fe5e979 100644
> --- a/arch/powerpc/lib/test_emulate_step.c
> +++ b/arch/powerpc/lib/test_emulate_step.c
> @@ -53,6 +53,8 @@
>  	ppc_inst_prefix(PPC_PREFIX_MLS | __PPC_PRFX_R(pr) | IMM_H(i), \
>  			PPC_RAW_ADDI(t, a, i))
>  
> +#define TEST_SETB(t, bfa)       ppc_inst(PPC_INST_SETB | ___PPC_RT(t) | ___PPC_RA((bfa & 0x7) << 2))
> +
>  
>  static void __init init_pt_regs(struct pt_regs *regs)
>  {
> @@ -929,6 +931,33 @@ static struct compute_test compute_tests[] = {
>  			}
>  		}
>  	},
> +	{
> +		.mnemonic = "setb",
> +		.cpu_feature = CPU_FTR_ARCH_300,
> +		.subtests = {
> +			{
> +				.descr = "BFA = 1, CR = GT",
> +				.instr = TEST_SETB(20, 1),
> +				.regs = {
> +					.ccr = 0x4000000,
> +				}
> +			},
> +			{
> +				.descr = "BFA = 4, CR = LT",
> +				.instr = TEST_SETB(20, 4),
> +				.regs = {
> +					.ccr = 0x8000,
> +				}
> +			},
> +			{
> +				.descr = "BFA = 5, CR = EQ",
> +				.instr = TEST_SETB(20, 5),
> +				.regs = {
> +					.ccr = 0x200,
> +				}
> +			}
> +		}
> +	},
>  	{
>  		.mnemonic = "add",
>  		.subtests = {
> -- 
> 2.16.4
> 
> 

^ permalink raw reply

* Re: [PATCH v2 1/2] powerpc/sstep: Add emulation support for ‘setb’ instruction
From: Naveen N. Rao @ 2021-05-17 13:34 UTC (permalink / raw)
  To: linuxppc-dev, Sathvika Vasireddy; +Cc: dja
In-Reply-To: <7b735b0c898da0db2af8628a64df2f5114596f22.1620727160.git.sathvika@linux.vnet.ibm.com>

Sathvika Vasireddy wrote:
> This adds emulation support for the following instruction:
>    * Set Boolean (setb)
> 
> Signed-off-by: Sathvika Vasireddy <sathvika@linux.vnet.ibm.com>
> ---
>  arch/powerpc/lib/sstep.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)

Tested-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>

> 
> diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
> index 45bda2520755..aee42bcc775b 100644
> --- a/arch/powerpc/lib/sstep.c
> +++ b/arch/powerpc/lib/sstep.c
> @@ -1700,6 +1700,28 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
>  			op->val = regs->ccr & imm;
>  			goto compute_done;
>  
> +		case 128:	/* setb */
> +			if (!cpu_has_feature(CPU_FTR_ARCH_300))
> +				goto unknown_opcode;
> +			/*
> +			 * 'ra' encodes the CR field number (bfa) in the top 3 bits.
> +			 * Since each CR field is 4 bits,
> +			 * we can simply mask off the bottom two bits (bfa * 4)
> +			 * to yield the first bit in the CR field.
> +			 */
> +			ra = ra & ~0x3;
> +			/* 'val' stores bits of the CR field (bfa) */
> +			val = regs->ccr >> (CR0_SHIFT - ra);
> +			/* checks if the LT bit of CR field (bfa) is set */
> +			if (val & 8)
> +				op->val = -1;
> +			/* checks if the GT bit of CR field (bfa) is set */
> +			else if (val & 4)
> +				op->val = 1;
> +			else
> +				op->val = 0;
> +			goto compute_done;
> +
>  		case 144:	/* mtcrf */
>  			op->type = COMPUTE + SETCC;
>  			imm = 0xf0000000UL;
> -- 
> 2.16.4
> 
> 

^ permalink raw reply

* Re: Fwd: [Bug 213069] New: kernel BUG at arch/powerpc/include/asm/book3s/64/hash-4k.h:147! Oops: Exception in kernel mode, sig: 5 [#1]
From: Anshuman Khandual @ 2021-05-17 13:12 UTC (permalink / raw)
  To: Christophe Leroy, Aneesh Kumar K.V, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <52ab4861-3a77-14da-1179-3ea79964c950@csgroup.eu>



On 5/17/21 6:29 PM, Christophe Leroy wrote:
> 
> 
> Le 17/05/2021 à 14:49, Anshuman Khandual a écrit :
>>
>>
>> On 5/17/21 11:25 AM, Aneesh Kumar K.V wrote:
>>> On 5/17/21 11:17 AM, Christophe Leroy wrote:
>>>> +aneesh
>>>> +linuxppc-dev list
>>>>
>>>> Le 17/05/2021 à 07:44, Anshuman Khandual a écrit :
>>>>> Hello Christophe,
>>>>>
>>>>> DEBUG_VM_PGTABLE has now been re-enabled on powerpc recently ? was not
>>>>> aware about this. From the error log, it failed explicitly on 4K page
>>>>> size hash config.
>>>>>
>>>>> static inline pmd_t hash__pmd_mkhuge(pmd_t pmd)
>>>>> {
>>>>>           BUG();        ------> Failed
>>>>>           return pmd;
>>>>> }
>>>>>
>>>>> static inline pmd_t __pmd_mkhuge(pmd_t pmd)
>>>>> {
>>>>>           if (radix_enabled())
>>>>>                   return radix__pmd_mkhuge(pmd);
>>>>>           return hash__pmd_mkhuge(pmd);
>>>>> }
>>>>>
>>>>> pmd_t pfn_pmd(unsigned long pfn, pgprot_t pgprot)
>>>>> {
>>>>>           unsigned long pmdv;
>>>>>
>>>>>           pmdv = (pfn << PAGE_SHIFT) & PTE_RPN_MASK;
>>>>>
>>>>>           return __pmd_mkhuge(pmd_set_protbits(__pmd(pmdv), pgprot));
>>>>> }
>>>>>
>>>>> It seems like on powerpc, where pfn_pmd() makes a huge page but which
>>>>> is not supported on 4K hash config thus triggering the BUG(). But all
>>>>> pfn_pmd() call sites inside the debug_vm_pgtable() test are protected
>>>>> with CONFIG_TRANSPARENT_HUGEPAGE. IIUC unlike powerpc, pfn_pmd() does
>>>>> not directly make a huge page on other platforms.
>>>>>
>>>>> Looking at arch/powerpc/include/asm/book3s/64/hash-4k.h, all relevant
>>>>> THP helpers has BUG() or 0 which indicates THP might not be supported
>>>>> on 4K page size hash config ?
>>>>>
>>>>> But looking at arch/powerpc/platforms/Kconfig.cputype, it seems like
>>>>> HAVE_ARCH_TRANSPARENT_HUGEPAGE is invariably selected on PPC_BOOK3S_64
>>>>> platforms which I assume includes 4K page size hash config as well.
>>>>>
>>>>> Is THP some how getting enabled on this 4K page size hash config where
>>>>> it should not be (thus triggering the BUG) ? OR am I missing something
>>>>> here.
>>>>>
>>>>
>>>
>>> We should put those  pfn_pmd()  and pfn_pud() after
>>>
>>>      if (!has_transparent_hugepage())
>>>          return;
>>
>> The following patch has been lightly tested on arm64 and x86 platforms.
>> Could you please verify if this solves the problem on powerpc 4K hash
>> config ? Thank you.
> 
> No need to update pmd_advanced_tests() and pud_advanced_tests() ?

They already have has_transparent_hugepage() check and all pfn_pxx()
instances are after that check. Do you see any other concern ?

> 
> Christophe

^ permalink raw reply

* Re: Fwd: [Bug 213069] New: kernel BUG at arch/powerpc/include/asm/book3s/64/hash-4k.h:147! Oops: Exception in kernel mode, sig: 5 [#1]
From: Christophe Leroy @ 2021-05-17 12:59 UTC (permalink / raw)
  To: Anshuman Khandual, Aneesh Kumar K.V,
	linuxppc-dev@lists.ozlabs.org
In-Reply-To: <6fcdf4f2-9bed-4b0f-8ed8-74cb25484ea6@arm.com>



Le 17/05/2021 à 14:49, Anshuman Khandual a écrit :
> 
> 
> On 5/17/21 11:25 AM, Aneesh Kumar K.V wrote:
>> On 5/17/21 11:17 AM, Christophe Leroy wrote:
>>> +aneesh
>>> +linuxppc-dev list
>>>
>>> Le 17/05/2021 à 07:44, Anshuman Khandual a écrit :
>>>> Hello Christophe,
>>>>
>>>> DEBUG_VM_PGTABLE has now been re-enabled on powerpc recently ? was not
>>>> aware about this. From the error log, it failed explicitly on 4K page
>>>> size hash config.
>>>>
>>>> static inline pmd_t hash__pmd_mkhuge(pmd_t pmd)
>>>> {
>>>>           BUG();        ------> Failed
>>>>           return pmd;
>>>> }
>>>>
>>>> static inline pmd_t __pmd_mkhuge(pmd_t pmd)
>>>> {
>>>>           if (radix_enabled())
>>>>                   return radix__pmd_mkhuge(pmd);
>>>>           return hash__pmd_mkhuge(pmd);
>>>> }
>>>>
>>>> pmd_t pfn_pmd(unsigned long pfn, pgprot_t pgprot)
>>>> {
>>>>           unsigned long pmdv;
>>>>
>>>>           pmdv = (pfn << PAGE_SHIFT) & PTE_RPN_MASK;
>>>>
>>>>           return __pmd_mkhuge(pmd_set_protbits(__pmd(pmdv), pgprot));
>>>> }
>>>>
>>>> It seems like on powerpc, where pfn_pmd() makes a huge page but which
>>>> is not supported on 4K hash config thus triggering the BUG(). But all
>>>> pfn_pmd() call sites inside the debug_vm_pgtable() test are protected
>>>> with CONFIG_TRANSPARENT_HUGEPAGE. IIUC unlike powerpc, pfn_pmd() does
>>>> not directly make a huge page on other platforms.
>>>>
>>>> Looking at arch/powerpc/include/asm/book3s/64/hash-4k.h, all relevant
>>>> THP helpers has BUG() or 0 which indicates THP might not be supported
>>>> on 4K page size hash config ?
>>>>
>>>> But looking at arch/powerpc/platforms/Kconfig.cputype, it seems like
>>>> HAVE_ARCH_TRANSPARENT_HUGEPAGE is invariably selected on PPC_BOOK3S_64
>>>> platforms which I assume includes 4K page size hash config as well.
>>>>
>>>> Is THP some how getting enabled on this 4K page size hash config where
>>>> it should not be (thus triggering the BUG) ? OR am I missing something
>>>> here.
>>>>
>>>
>>
>> We should put those  pfn_pmd()  and pfn_pud() after
>>
>>      if (!has_transparent_hugepage())
>>          return;
> 
> The following patch has been lightly tested on arm64 and x86 platforms.
> Could you please verify if this solves the problem on powerpc 4K hash
> config ? Thank you.

No need to update pmd_advanced_tests() and pud_advanced_tests() ?

Christophe

^ permalink raw reply

* [Bug 213069] kernel BUG at arch/powerpc/include/asm/book3s/64/hash-4k.h:147! Oops: Exception in kernel mode, sig: 5 [#1]
From: bugzilla-daemon @ 2021-05-17 12:57 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <bug-213069-206035@https.bugzilla.kernel.org/>

https://bugzilla.kernel.org/show_bug.cgi?id=213069

--- Comment #3 from Christophe Leroy (christophe.leroy@csgroup.eu) ---
Patch proposed by Anshuman, to be tested:

https://lore.kernel.org/linuxppc-dev/bug-213069-206035@https.bugzilla.kernel.org%2F/T/#m82f2974b813c4d3a085b0cd2ac3d5b732a362e68

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.

^ permalink raw reply

* Re: Fwd: [Bug 213069] New: kernel BUG at arch/powerpc/include/asm/book3s/64/hash-4k.h:147! Oops: Exception in kernel mode, sig: 5 [#1]
From: Anshuman Khandual @ 2021-05-17 12:49 UTC (permalink / raw)
  To: Aneesh Kumar K.V, Christophe Leroy, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <e9558e0a-314e-ddfd-6776-84c1bfe6f01f@linux.ibm.com>



On 5/17/21 11:25 AM, Aneesh Kumar K.V wrote:
> On 5/17/21 11:17 AM, Christophe Leroy wrote:
>> +aneesh
>> +linuxppc-dev list
>>
>> Le 17/05/2021 à 07:44, Anshuman Khandual a écrit :
>>> Hello Christophe,
>>>
>>> DEBUG_VM_PGTABLE has now been re-enabled on powerpc recently ? was not
>>> aware about this. From the error log, it failed explicitly on 4K page
>>> size hash config.
>>>
>>> static inline pmd_t hash__pmd_mkhuge(pmd_t pmd)
>>> {
>>>          BUG();        ------> Failed
>>>          return pmd;
>>> }
>>>
>>> static inline pmd_t __pmd_mkhuge(pmd_t pmd)
>>> {
>>>          if (radix_enabled())
>>>                  return radix__pmd_mkhuge(pmd);
>>>          return hash__pmd_mkhuge(pmd);
>>> }
>>>
>>> pmd_t pfn_pmd(unsigned long pfn, pgprot_t pgprot)
>>> {
>>>          unsigned long pmdv;
>>>
>>>          pmdv = (pfn << PAGE_SHIFT) & PTE_RPN_MASK;
>>>
>>>          return __pmd_mkhuge(pmd_set_protbits(__pmd(pmdv), pgprot));
>>> }
>>>
>>> It seems like on powerpc, where pfn_pmd() makes a huge page but which
>>> is not supported on 4K hash config thus triggering the BUG(). But all
>>> pfn_pmd() call sites inside the debug_vm_pgtable() test are protected
>>> with CONFIG_TRANSPARENT_HUGEPAGE. IIUC unlike powerpc, pfn_pmd() does
>>> not directly make a huge page on other platforms.
>>>
>>> Looking at arch/powerpc/include/asm/book3s/64/hash-4k.h, all relevant
>>> THP helpers has BUG() or 0 which indicates THP might not be supported
>>> on 4K page size hash config ?
>>>
>>> But looking at arch/powerpc/platforms/Kconfig.cputype, it seems like
>>> HAVE_ARCH_TRANSPARENT_HUGEPAGE is invariably selected on PPC_BOOK3S_64
>>> platforms which I assume includes 4K page size hash config as well.
>>>
>>> Is THP some how getting enabled on this 4K page size hash config where
>>> it should not be (thus triggering the BUG) ? OR am I missing something
>>> here.
>>>
>>
> 
> We should put those  pfn_pmd()  and pfn_pud() after
> 
>     if (!has_transparent_hugepage())
>         return;

The following patch has been lightly tested on arm64 and x86 platforms.
Could you please verify if this solves the problem on powerpc 4K hash
config ? Thank you.

- Anshuman

 mm/debug_vm_pgtable.c | 58 +++++++++++++++++++++++++++++++++++--------
 1 file changed, 48 insertions(+), 10 deletions(-)

diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
index e2f35db8ba69..6ff92c8b0a00 100644
--- a/mm/debug_vm_pgtable.c
+++ b/mm/debug_vm_pgtable.c
@@ -146,13 +146,14 @@ static void __init pte_savedwrite_tests(unsigned long pfn, pgprot_t prot)
 static void __init pmd_basic_tests(unsigned long pfn, int idx)
 {
 	pgprot_t prot = protection_map[idx];
-	pmd_t pmd = pfn_pmd(pfn, prot);
 	unsigned long val = idx, *ptr = &val;
+	pmd_t pmd;
 
 	if (!has_transparent_hugepage())
 		return;
 
 	pr_debug("Validating PMD basic (%pGv)\n", ptr);
+	pmd = pfn_pmd(pfn, prot);
 
 	/*
 	 * This test needs to be executed after the given page table entry
@@ -232,9 +233,14 @@ static void __init pmd_advanced_tests(struct mm_struct *mm,
 
 static void __init pmd_leaf_tests(unsigned long pfn, pgprot_t prot)
 {
-	pmd_t pmd = pfn_pmd(pfn, prot);
+	pmd_t pmd;
+
+	if (!has_transparent_hugepage())
+		return;
 
 	pr_debug("Validating PMD leaf\n");
+	pmd = pfn_pmd(pfn, prot);
+
 	/*
 	 * PMD based THP is a leaf entry.
 	 */
@@ -244,12 +250,16 @@ static void __init pmd_leaf_tests(unsigned long pfn, pgprot_t prot)
 
 static void __init pmd_savedwrite_tests(unsigned long pfn, pgprot_t prot)
 {
-	pmd_t pmd = pfn_pmd(pfn, prot);
+	pmd_t pmd;
 
 	if (!IS_ENABLED(CONFIG_NUMA_BALANCING))
 		return;
 
+	if (!has_transparent_hugepage())
+		return;
+
 	pr_debug("Validating PMD saved write\n");
+	pmd = pfn_pmd(pfn, prot);
 	WARN_ON(!pmd_savedwrite(pmd_mk_savedwrite(pmd_clear_savedwrite(pmd))));
 	WARN_ON(pmd_savedwrite(pmd_clear_savedwrite(pmd_mk_savedwrite(pmd))));
 }
@@ -258,13 +268,14 @@ static void __init pmd_savedwrite_tests(unsigned long pfn, pgprot_t prot)
 static void __init pud_basic_tests(struct mm_struct *mm, unsigned long pfn, int idx)
 {
 	pgprot_t prot = protection_map[idx];
-	pud_t pud = pfn_pud(pfn, prot);
 	unsigned long val = idx, *ptr = &val;
+	pud_t pud;
 
 	if (!has_transparent_hugepage())
 		return;
 
 	pr_debug("Validating PUD basic (%pGv)\n", ptr);
+	pud = pfn_pud(pfn, prot);
 
 	/*
 	 * This test needs to be executed after the given page table entry
@@ -348,9 +359,13 @@ static void __init pud_advanced_tests(struct mm_struct *mm,
 
 static void __init pud_leaf_tests(unsigned long pfn, pgprot_t prot)
 {
-	pud_t pud = pfn_pud(pfn, prot);
+	pud_t pud;
+
+	if (!has_transparent_hugepage())
+		return;
 
 	pr_debug("Validating PUD leaf\n");
+	pud = pfn_pud(pfn, prot);
 	/*
 	 * PUD based THP is a leaf entry.
 	 */
@@ -642,12 +657,16 @@ static void __init pte_protnone_tests(unsigned long pfn, pgprot_t prot)
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 static void __init pmd_protnone_tests(unsigned long pfn, pgprot_t prot)
 {
-	pmd_t pmd = pmd_mkhuge(pfn_pmd(pfn, prot));
+	pmd_t pmd;
 
 	if (!IS_ENABLED(CONFIG_NUMA_BALANCING))
 		return;
 
+	if (!has_transparent_hugepage())
+		return;
+
 	pr_debug("Validating PMD protnone\n");
+	pmd = pmd_mkhuge(pfn_pmd(pfn, prot));
 	WARN_ON(!pmd_protnone(pmd));
 	WARN_ON(!pmd_present(pmd));
 }
@@ -667,18 +686,26 @@ static void __init pte_devmap_tests(unsigned long pfn, pgprot_t prot)
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 static void __init pmd_devmap_tests(unsigned long pfn, pgprot_t prot)
 {
-	pmd_t pmd = pfn_pmd(pfn, prot);
+	pmd_t pmd;
+
+	if (!has_transparent_hugepage())
+		return;
 
 	pr_debug("Validating PMD devmap\n");
+	pmd = pfn_pmd(pfn, prot);
 	WARN_ON(!pmd_devmap(pmd_mkdevmap(pmd)));
 }
 
 #ifdef CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD
 static void __init pud_devmap_tests(unsigned long pfn, pgprot_t prot)
 {
-	pud_t pud = pfn_pud(pfn, prot);
+	pud_t pud;
+
+	if (!has_transparent_hugepage())
+		return;
 
 	pr_debug("Validating PUD devmap\n");
+	pud = pfn_pud(pfn, prot);
 	WARN_ON(!pud_devmap(pud_mkdevmap(pud)));
 }
 #else  /* !CONFIG_HAVE_ARCH_TRANSPARENT_HUGEPAGE_PUD */
@@ -721,25 +748,33 @@ static void __init pte_swap_soft_dirty_tests(unsigned long pfn, pgprot_t prot)
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 static void __init pmd_soft_dirty_tests(unsigned long pfn, pgprot_t prot)
 {
-	pmd_t pmd = pfn_pmd(pfn, prot);
+	pmd_t pmd;
 
 	if (!IS_ENABLED(CONFIG_MEM_SOFT_DIRTY))
 		return;
 
+	if (!has_transparent_hugepage())
+		return;
+
 	pr_debug("Validating PMD soft dirty\n");
+	pmd = pfn_pmd(pfn, prot);
 	WARN_ON(!pmd_soft_dirty(pmd_mksoft_dirty(pmd)));
 	WARN_ON(pmd_soft_dirty(pmd_clear_soft_dirty(pmd)));
 }
 
 static void __init pmd_swap_soft_dirty_tests(unsigned long pfn, pgprot_t prot)
 {
-	pmd_t pmd = pfn_pmd(pfn, prot);
+	pmd_t pmd;
 
 	if (!IS_ENABLED(CONFIG_MEM_SOFT_DIRTY) ||
 		!IS_ENABLED(CONFIG_ARCH_ENABLE_THP_MIGRATION))
 		return;
 
+	if (!has_transparent_hugepage())
+		return;
+
 	pr_debug("Validating PMD swap soft dirty\n");
+	pmd = pfn_pmd(pfn, prot);
 	WARN_ON(!pmd_swp_soft_dirty(pmd_swp_mksoft_dirty(pmd)));
 	WARN_ON(pmd_swp_soft_dirty(pmd_swp_clear_soft_dirty(pmd)));
 }
@@ -768,6 +803,9 @@ static void __init pmd_swap_tests(unsigned long pfn, pgprot_t prot)
 	swp_entry_t swp;
 	pmd_t pmd;
 
+	if (!has_transparent_hugepage())
+		return;
+
 	pr_debug("Validating PMD swap\n");
 	pmd = pfn_pmd(pfn, prot);
 	swp = __pmd_to_swp_entry(pmd);
-- 
2.20.1


^ permalink raw reply related

* [PATCH 1/1] selftests/powerpc: Remove duplicated header file inclusion
From: Zhen Lei @ 2021-05-17 12:16 UTC (permalink / raw)
  To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
	Shuah Khan, linuxppc-dev, linux-kselftest
  Cc: Zhen Lei

The header file <pthread.h> is already included above and can be
removed here.

Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
---
 tools/testing/selftests/powerpc/tm/tm-vmx-unavail.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tools/testing/selftests/powerpc/tm/tm-vmx-unavail.c b/tools/testing/selftests/powerpc/tm/tm-vmx-unavail.c
index e2a0c07e8362..9ef37a9836ac 100644
--- a/tools/testing/selftests/powerpc/tm/tm-vmx-unavail.c
+++ b/tools/testing/selftests/powerpc/tm/tm-vmx-unavail.c
@@ -17,7 +17,6 @@
 #include <pthread.h>
 #include <sys/mman.h>
 #include <unistd.h>
-#include <pthread.h>
 
 #include "tm.h"
 #include "utils.h"
-- 
2.25.1



^ permalink raw reply related

* Re: [PATCH] watchdog: Remove MV64x60 watchdog driver
From: Michael Ellerman @ 2021-05-17 11:17 UTC (permalink / raw)
  To: Guenter Roeck, Christophe Leroy, Wim Van Sebroeck,
	Sebastian Hesselbarth
  Cc: netdev, linuxppc-dev, linux-kernel, linux-watchdog
In-Reply-To: <31d702e5-22d1-1766-76dd-e24860e5b1a4@roeck-us.net>

Guenter Roeck <linux@roeck-us.net> writes:
> On 3/18/21 10:25 AM, Christophe Leroy wrote:
>> Commit 92c8c16f3457 ("powerpc/embedded6xx: Remove C2K board support")
>> removed the last selector of CONFIG_MV64X60.
>> 
>> Therefore CONFIG_MV64X60_WDT cannot be selected anymore and
>> can be removed.
>> 
>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
>
>> ---
>>  drivers/watchdog/Kconfig       |   4 -
>>  drivers/watchdog/Makefile      |   1 -
>>  drivers/watchdog/mv64x60_wdt.c | 324 ---------------------------------
>>  include/linux/mv643xx.h        |   8 -
>>  4 files changed, 337 deletions(-)
>>  delete mode 100644 drivers/watchdog/mv64x60_wdt.c

I assumed this would go via the watchdog tree, but seems like I
misinterpreted.

Should I take this via the powerpc tree for v5.14 ?

cheers

^ permalink raw reply

* Re: Fwd: [Bug 213069] New: kernel BUG at arch/powerpc/include/asm/book3s/64/hash-4k.h:147! Oops: Exception in kernel mode, sig: 5 [#1]
From: Aneesh Kumar K.V @ 2021-05-17 11:14 UTC (permalink / raw)
  To: Anshuman Khandual, Christophe Leroy,
	linuxppc-dev@lists.ozlabs.org
In-Reply-To: <18508d23-3884-af84-ce8f-65b31ac83192@arm.com>

On 5/17/21 4:23 PM, Anshuman Khandual wrote:
> 
> 
> On 5/17/21 1:14 PM, Aneesh Kumar K.V wrote:
>> On 5/17/21 12:55 PM, Anshuman Khandual wrote:
>>>
>>>
>>> On 5/17/21 11:25 AM, Aneesh Kumar K.V wrote:
>>>> On 5/17/21 11:17 AM, Christophe Leroy wrote:
>>>>> +aneesh
>>>>> +linuxppc-dev list
>>>>>
>>>>> Le 17/05/2021 à 07:44, Anshuman Khandual a écrit :
>>>>>> Hello Christophe,
>>>>>>
>>>>>> DEBUG_VM_PGTABLE has now been re-enabled on powerpc recently ? was not
>>>>>> aware about this. From the error log, it failed explicitly on 4K page
>>>>>> size hash config.
>>>>>>
>>>>>> static inline pmd_t hash__pmd_mkhuge(pmd_t pmd)
>>>>>> {
>>>>>>            BUG();        ------> Failed
>>>>>>            return pmd;
>>>>>> }
>>>>>>
>>>>>> static inline pmd_t __pmd_mkhuge(pmd_t pmd)
>>>>>> {
>>>>>>            if (radix_enabled())
>>>>>>                    return radix__pmd_mkhuge(pmd);
>>>>>>            return hash__pmd_mkhuge(pmd);
>>>>>> }
>>>>>>
>>>>>> pmd_t pfn_pmd(unsigned long pfn, pgprot_t pgprot)
>>>>>> {
>>>>>>            unsigned long pmdv;
>>>>>>
>>>>>>            pmdv = (pfn << PAGE_SHIFT) & PTE_RPN_MASK;
>>>>>>
>>>>>>            return __pmd_mkhuge(pmd_set_protbits(__pmd(pmdv), pgprot));
>>>>>> }
>>>>>>
>>>>>> It seems like on powerpc, where pfn_pmd() makes a huge page but which
>>>>>> is not supported on 4K hash config thus triggering the BUG(). But all
>>>>>> pfn_pmd() call sites inside the debug_vm_pgtable() test are protected
>>>>>> with CONFIG_TRANSPARENT_HUGEPAGE. IIUC unlike powerpc, pfn_pmd() does
>>>>>> not directly make a huge page on other platforms.
>>>>>>
>>>>>> Looking at arch/powerpc/include/asm/book3s/64/hash-4k.h, all relevant
>>>>>> THP helpers has BUG() or 0 which indicates THP might not be supported
>>>>>> on 4K page size hash config ?
>>>>>>
>>>>>> But looking at arch/powerpc/platforms/Kconfig.cputype, it seems like
>>>>>> HAVE_ARCH_TRANSPARENT_HUGEPAGE is invariably selected on PPC_BOOK3S_64
>>>>>> platforms which I assume includes 4K page size hash config as well.
>>>>>>
>>>>>> Is THP some how getting enabled on this 4K page size hash config where
>>>>>> it should not be (thus triggering the BUG) ? OR am I missing something
>>>>>> here.
>>>>>>
>>>>>
>>>>
>>>> We should put those  pfn_pmd()  and pfn_pud() after
>>>>
>>>>       if (!has_transparent_hugepage())
>>>>           return;
>>>>
>>>>
>>>> On hash with 4K page size, we can't support leaf page table entry and PMD and PUD level. Hence we don't support THP for them.
>>>
>>> But could CONFIG_TRANSPARENT_HUGEPAGE be enabled on such configs ?
>>> Otherwise, wondering how the BUG() just got triggered there.
>>>
>>
>> yes. We do support THP with radix translation and 4K page size.
> 
> I was actually wondering about whether 4K page size in hash config
> supports THP and could enable CONFIG_TRANSPARENT_HUGEPAGE ? But as
> there is a BUG() trigger on hash__pmd_mkhuge(), it seems like THP
> config should not have been enabled for such platforms but it does
> for some reason. Could not we disable it ?
> 

Hash is a translation mode which is not build time selected. Rather it 
is runtime selected. Same kernel can boot into either hash or radix 
translation. We don't support THP with 4K pagesize with hash 
translation.But the same kernel when booted in radix translation mode 
will support THP.

-aneesh

^ permalink raw reply

* Re: [PATCH v14 3/9] powerpc/modules: Make module_alloc() Strict Module RWX aware
From: Christophe Leroy @ 2021-05-17 11:05 UTC (permalink / raw)
  To: Michael Ellerman, Jordan Niethe
  Cc: ajd, Aneesh Kumar K.V, Nicholas Piggin, cmr, naveen.n.rao,
	linuxppc-dev, Daniel Axtens
In-Reply-To: <87lf8dk4iw.fsf@mpe.ellerman.id.au>



Le 17/05/2021 à 13:01, Michael Ellerman a écrit :
> Jordan Niethe <jniethe5@gmail.com> writes:
>> On Mon, May 17, 2021 at 4:37 PM Christophe Leroy
>> <christophe.leroy@csgroup.eu> wrote:
>>> Le 17/05/2021 à 05:28, Jordan Niethe a écrit :
>>>> Make module_alloc() use PAGE_KERNEL protections instead of
>>>> PAGE_KERNEL_EXEX if Strict Module RWX is enabled.
>>>>
>>>> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
>>>> ---
>>>> v14: - Split out from powerpc: Set ARCH_HAS_STRICT_MODULE_RWX
>>>>        - Add and use strict_module_rwx_enabled() helper
>>>> ---
>>>>    arch/powerpc/include/asm/mmu.h | 5 +++++
>>>>    arch/powerpc/kernel/module.c   | 4 +++-
>>>>    2 files changed, 8 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/powerpc/include/asm/mmu.h b/arch/powerpc/include/asm/mmu.h
>>>> index 607168b1aef4..7710bf0cbf8a 100644
>>>> --- a/arch/powerpc/include/asm/mmu.h
>>>> +++ b/arch/powerpc/include/asm/mmu.h
>>>> @@ -357,6 +357,11 @@ static inline bool strict_kernel_rwx_enabled(void)
>>>>        return false;
>>>>    }
>>>>    #endif
>>>> +
>>>> +static inline bool strict_module_rwx_enabled(void)
>>>> +{
>>>> +     return IS_ENABLED(CONFIG_STRICT_MODULE_RWX) && strict_kernel_rwx_enabled();
>>>> +}
>>>
>>> Looking at arch/Kconfig, I have the feeling that it is possible to select CONFIG_STRICT_MODULE_RWX
>>> without selecting CONFIG_STRICT_KERNEL_RWX.
>>>
>>> In that case, strict_kernel_rwx_enabled() will return false.
> 
>> Ok, if someone did that currently it would break things, e.g. code
>> patching. I think it should it be made impossible to
>> CONFIG_STRICT_MODULE_RWX without CONFIG_STRICT_KERNEL_RWX?
> 
> Yeah I don't see any reason to support that combination.
> 
> We should be moving to a world where both are on by default, or in fact
> are always enabled.
> 

Would it work if we add the following in arch/powerpc/Kconfig ? :

	select STRICT_KERNEL_RWX if STRICT_MODULE_RWX

There should be no dependency issue as powerpc only selects ARCH_HAS_STRICT_MODULE_RWX when 
ARCH_HAS_STRICT_KERNEL_RWX is also selected.

Christophe

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox